feat: implement basic keystore persistence and CLI wallet commands#112
feat: implement basic keystore persistence and CLI wallet commands#112SIDDHANTCOOKIE wants to merge 24 commits into
Conversation
Feat/ transaction receipts
Feat/smart contracts
Readme update
Feat/fork choice
feat: implement JSON-RPC 2.0 server using aiohttp
Feat/contract transfers
Feat/libp2p
minichain ascii art
feat: implemented (EMA) difficulty adjustment
Feat/peer blacklisting
Feat/difficulty
Feat/libp2p
Signed-off-by: g-k-s-03 <govindsingh97704@gmail.com>
Signed-off-by: g-k-s-03 <govindsingh97704@gmail.com>
…lean bug and apply_transaction malformed guard Signed-off-by: g-k-s-03 <govindsingh97704@gmail.com>
Signed-off-by: g-k-s-03 <govindsingh97704@gmail.com>
Signed-off-by: g-k-s-03 <govindsingh97704@gmail.com>
Signed-off-by: g-k-s-03 <govindsingh97704@gmail.com>
…unters, defer mark_seen, cap buffer size Signed-off-by: g-k-s-03 <govindsingh97704@gmail.com>
WalkthroughAdds filesystem-backed wallet keystore loading/creation, propagates ChangesNode, wallet, CLI, and P2P validation/misbehavior changes
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Peer
participant TrioStream
participant AsyncioReader
participant P2PNetwork
participant Chain
Peer->>TrioStream: send message frame
TrioStream->>TrioStream: check is_peer_banned
TrioStream->>AsyncioReader: forward parsed/malformed message
AsyncioReader->>Chain: process tx/block
Chain-->>AsyncioReader: ValidationStatus
AsyncioReader->>P2PNetwork: _handle_validation_status(status)
P2PNetwork->>P2PNetwork: increment counter / check threshold
P2PNetwork->>Peer: ban_peer + disconnect_peer
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ 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: 7
🤖 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 510-524: The key import path duplicates the same keystore path
creation and JSON write logic already used in load_or_create_wallet, so extract
that shared persistence into a save_keystore(sk, path) helper and call it from
both places. Move the os.makedirs and json.dump handling into the helper, then
reuse it in the import flow after new_sk is created so there is one place to
apply the permission-safe keystore write behavior.
- Around line 70-76: The keystore is being written with default process
permissions, which can leave the private key readable by other users; update the
wallet creation path and the import_key write path to explicitly create the file
with owner-only permissions (0600). Use the existing keystore write logic around
the open(json.dump...) block and the corresponding import_key file write so both
paths set the file mode immediately after creation, and keep the current
logging/error handling intact.
- Around line 54-77: The wallet loading flow in the keystore handling logic can
overwrite an existing `keystore.json` when `load_wallet` fails to read valid
data and then falls through to the generation path. Update the control flow so
that if `os.path.exists(keystore_path)` is true but `json.load`, `private_key`
parsing, or `SigningKey` reconstruction fails, the function refuses to create a
new wallet at the same path and instead raises/returns an error after logging.
Keep the existing success path for valid keystores, but ensure the new-wallet
branch only runs when no keystore file exists.
In `@minichain/p2p.py`:
- Around line 87-89: The background tasks started in the p2p node setup are
being created and then discarded, so `_asyncio_reader` and `_decay_counters` may
be garbage-collected while still running. Update the `P2P`/node initialization
flow to store the `asyncio.create_task(...)` results on `self` (or another
long-lived owner) and keep those references for the lifetime of the node,
alongside the existing `trio.run` thread startup.
- Around line 236-238: In the p2p validation flow, `add_block()` is treating
every `ValueError` mentioning “hash” as `INVALID`, which causes normal fork/sync
cases from `validate_block_link_and_hash()` (“invalid previous hash ...”) to be
misclassified and sent down the ban path in `_handle_validation_status()`.
Update the status mapping in `minichain/chain.py` to distinguish
link/previous-hash mismatches from real hash corruption, and only return
`INVALID` for the latter; then keep `minichain/p2p.py`’s
`_handle_validation_status()` logic unchanged so forked/ahead blocks are not
immediately banned.
- Around line 303-305: The fixed MAX_FRAME_BYTES check in p2p.py is too small
for large chain sync responses and can incorrectly send peers to the MALFORMED
path. Update the framing logic around the buffer length check in the receive
loop, and either increase the limit or change the chain sync flow so
`chain_response` from `chain_request` is chunked/bounded before it reaches the
parser. Make sure the fix preserves normal framing while preventing valid
full-reorg responses from being disconnected.
In `@minichain/state.py`:
- Around line 94-100: The amount/fee checks in validate_and_apply are duplicated
in apply_transaction and the other entry point, so remove the redundant guards
here and let the shared validation path handle them, or extract a single
_validate_amount_fee(tx) helper and call it from validate_and_apply,
validate_and_apply_with_status, and apply_transaction to keep the semantic
contract in one place.
🪄 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: 14de8299-e224-469a-b57b-db3ac4a0cc07
📒 Files selected for processing (6)
main.pyminichain/chain.pyminichain/p2p.pyminichain/state.pytests/test_difficulty.pytests/test_persistence_runtime.py
| if os.path.exists(keystore_path): | ||
| try: | ||
| with open(keystore_path, "r") as f: | ||
| data = json.load(f) | ||
| sk_hex = data.get("private_key") | ||
| if sk_hex: | ||
| sk = SigningKey(bytes.fromhex(sk_hex)) | ||
| pk = sk.verify_key.encode(encoder=HexEncoder).decode() | ||
| logger.info("Loaded existing wallet from %s", keystore_path) | ||
| return sk, pk | ||
| except Exception as e: | ||
| logger.warning("Failed to load keystore: %s", e) | ||
|
|
||
| sk = SigningKey.generate() | ||
| pk = sk.verify_key.encode(encoder=HexEncoder).decode() | ||
|
|
||
| os.makedirs(path, exist_ok=True) | ||
| try: | ||
| with open(keystore_path, "w") as f: | ||
| json.dump({"private_key": sk.encode(encoder=HexEncoder).decode()}, f) | ||
| logger.info("Created new wallet at %s", keystore_path) | ||
| except Exception as e: | ||
| logger.warning("Failed to save keystore: %s", e) | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | 🏗️ Heavy lift
Do not silently overwrite an existing keystore when loading fails — this destroys the private key.
If keystore.json exists but the JSON is corrupt, unreadable (e.g. transient permission error), or the private_key field is missing/empty, control falls through the try block and reaches the generation path, which then does open(keystore_path, "w") at the same path — permanently overwriting the only copy of the user's private key with a freshly generated one. For a wallet this is unrecoverable fund loss.
This mirrors the corrupted-chain case in run_node, which deliberately refuses to start rather than overwrite (lines 595-598). Apply the same guard here.
🛡️ Proposed fix: fail instead of overwriting
if os.path.exists(keystore_path):
try:
- with open(keystore_path, "r") as f:
+ with open(keystore_path) as f:
data = json.load(f)
sk_hex = data.get("private_key")
if sk_hex:
sk = SigningKey(bytes.fromhex(sk_hex))
pk = sk.verify_key.encode(encoder=HexEncoder).decode()
logger.info("Loaded existing wallet from %s", keystore_path)
return sk, pk
- except Exception as e:
- logger.warning("Failed to load keystore: %s", e)
+ raise ValueError("keystore.json present but missing 'private_key'")
+ except Exception as e:
+ logger.error("Failed to load existing keystore at %s: %s", keystore_path, e)
+ logger.error("Refusing to overwrite an existing keystore to avoid losing the private key.")
+ sys.exit(1)📝 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 os.path.exists(keystore_path): | |
| try: | |
| with open(keystore_path, "r") as f: | |
| data = json.load(f) | |
| sk_hex = data.get("private_key") | |
| if sk_hex: | |
| sk = SigningKey(bytes.fromhex(sk_hex)) | |
| pk = sk.verify_key.encode(encoder=HexEncoder).decode() | |
| logger.info("Loaded existing wallet from %s", keystore_path) | |
| return sk, pk | |
| except Exception as e: | |
| logger.warning("Failed to load keystore: %s", e) | |
| sk = SigningKey.generate() | |
| pk = sk.verify_key.encode(encoder=HexEncoder).decode() | |
| os.makedirs(path, exist_ok=True) | |
| try: | |
| with open(keystore_path, "w") as f: | |
| json.dump({"private_key": sk.encode(encoder=HexEncoder).decode()}, f) | |
| logger.info("Created new wallet at %s", keystore_path) | |
| except Exception as e: | |
| logger.warning("Failed to save keystore: %s", e) | |
| if os.path.exists(keystore_path): | |
| try: | |
| with open(keystore_path) as f: | |
| data = json.load(f) | |
| sk_hex = data.get("private_key") | |
| if sk_hex: | |
| sk = SigningKey(bytes.fromhex(sk_hex)) | |
| pk = sk.verify_key.encode(encoder=HexEncoder).decode() | |
| logger.info("Loaded existing wallet from %s", keystore_path) | |
| return sk, pk | |
| raise ValueError("keystore.json present but missing 'private_key'") | |
| except Exception as e: | |
| logger.error("Failed to load existing keystore at %s: %s", keystore_path, e) | |
| logger.error("Refusing to overwrite an existing keystore to avoid losing the private key.") | |
| sys.exit(1) | |
| sk = SigningKey.generate() | |
| pk = sk.verify_key.encode(encoder=HexEncoder).decode() | |
| os.makedirs(path, exist_ok=True) | |
| try: | |
| with open(keystore_path, "w") as f: | |
| json.dump({"private_key": sk.encode(encoder=HexEncoder).decode()}, f) | |
| logger.info("Created new wallet at %s", keystore_path) | |
| except Exception as e: | |
| logger.warning("Failed to save keystore: %s", e) |
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] 55-55: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(keystore_path, "r")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
[warning] 71-71: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(keystore_path, "w")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
🪛 Ruff (0.15.20)
[warning] 56-56: Unnecessary mode argument
Remove mode argument
(UP015)
[warning] 64-64: Do not catch blind exception: Exception
(BLE001)
[warning] 75-75: Do not catch blind exception: Exception
(BLE001)
🤖 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 54 - 77, The wallet loading flow in the keystore
handling logic can overwrite an existing `keystore.json` when `load_wallet`
fails to read valid data and then falls through to the generation path. Update
the control flow so that if `os.path.exists(keystore_path)` is true but
`json.load`, `private_key` parsing, or `SigningKey` reconstruction fails, the
function refuses to create a new wallet at the same path and instead
raises/returns an error after logging. Keep the existing success path for valid
keystores, but ensure the new-wallet branch only runs when no keystore file
exists.
| os.makedirs(path, exist_ok=True) | ||
| try: | ||
| with open(keystore_path, "w") as f: | ||
| json.dump({"private_key": sk.encode(encoder=HexEncoder).decode()}, f) | ||
| logger.info("Created new wallet at %s", keystore_path) | ||
| except Exception as e: | ||
| logger.warning("Failed to save keystore: %s", e) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Restrict keystore file permissions to owner-only (0600).
The private key is written unencrypted (as the comment acknowledges) but the file is also created with the process umask default, typically world/group-readable. On a shared host any local user can read the key. Create it with mode 0600. The same applies to the import_key write at Line 517-518.
🔒 Proposed fix
os.makedirs(path, exist_ok=True)
try:
- with open(keystore_path, "w") as f:
- json.dump({"private_key": sk.encode(encoder=HexEncoder).decode()}, f)
+ fd = os.open(keystore_path, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600)
+ with os.fdopen(fd, "w") as f:
+ json.dump({"private_key": sk.encode(encoder=HexEncoder).decode()}, f)
logger.info("Created new wallet at %s", keystore_path)📝 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.
| os.makedirs(path, exist_ok=True) | |
| try: | |
| with open(keystore_path, "w") as f: | |
| json.dump({"private_key": sk.encode(encoder=HexEncoder).decode()}, f) | |
| logger.info("Created new wallet at %s", keystore_path) | |
| except Exception as e: | |
| logger.warning("Failed to save keystore: %s", e) | |
| os.makedirs(path, exist_ok=True) | |
| try: | |
| fd = os.open(keystore_path, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600) | |
| with os.fdopen(fd, "w") as f: | |
| json.dump({"private_key": sk.encode(encoder=HexEncoder).decode()}, f) | |
| logger.info("Created new wallet at %s", keystore_path) | |
| except Exception as e: | |
| logger.warning("Failed to save keystore: %s", e) |
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] 71-71: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(keystore_path, "w")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
🪛 Ruff (0.15.20)
[warning] 75-75: Do not catch blind exception: Exception
(BLE001)
🤖 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 70 - 76, The keystore is being written with default
process permissions, which can leave the private key readable by other users;
update the wallet creation path and the import_key write path to explicitly
create the file with owner-only permissions (0600). Use the existing keystore
write logic around the open(json.dump...) block and the corresponding import_key
file write so both paths set the file mode immediately after creation, and keep
the current logging/error handling intact.
| try: | ||
| new_sk = SigningKey(bytes.fromhex(parts[1])) | ||
| new_pk = new_sk.verify_key.encode(encoder=HexEncoder).decode() | ||
|
|
||
| path = datadir or "." | ||
| keystore_path = os.path.join(path, "keystore.json") | ||
| os.makedirs(path, exist_ok=True) | ||
| with open(keystore_path, "w") as f: | ||
| json.dump({"private_key": new_sk.encode(encoder=HexEncoder).decode()}, f) | ||
|
|
||
| sk, pk = new_sk, new_pk | ||
| print(f" {C_GREEN}✅ Key imported successfully!{C_RESET}") | ||
| print_prompt_info(pk) | ||
| except Exception as e: | ||
| print(f" {C_RED}❌ Failed to import key: {e}{C_RESET}") |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Extract a shared keystore-write helper to avoid duplication.
This block re-implements the keystore path/makedirs/json.dump write already present in load_or_create_wallet (lines 70-73). Extracting a save_keystore(sk, path) helper removes the duplication and lets the permission-safe write (0600, see other comment) be applied in one place for both call sites.
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] 516-516: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(keystore_path, "w")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
🪛 Ruff (0.15.20)
[warning] 517-517: Async functions should not open files with blocking methods like open
(ASYNC230)
[warning] 523-523: Do not catch blind exception: Exception
(BLE001)
🤖 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 510 - 524, The key import path duplicates the same
keystore path creation and JSON write logic already used in
load_or_create_wallet, so extract that shared persistence into a
save_keystore(sk, path) helper and call it from both places. Move the
os.makedirs and json.dump handling into the helper, then reuse it in the import
flow after new_sk is created so there is one place to apply the permission-safe
keystore write behavior.
| threading.Thread(target=trio.run, args=(self._trio_main,), daemon=True).start() | ||
| asyncio.create_task(self._asyncio_reader()) | ||
| asyncio.create_task(self._decay_counters()) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
Store references to the background tasks.
asyncio.create_task(...) return values are discarded here. CPython only keeps weak references to tasks, so _asyncio_reader and _decay_counters can be garbage-collected mid-flight, silently stopping message processing / counter decay. Retain references (e.g., on self) for the node lifetime.
♻️ Proposed fix
- threading.Thread(target=trio.run, args=(self._trio_main,), daemon=True).start()
- asyncio.create_task(self._asyncio_reader())
- asyncio.create_task(self._decay_counters())
+ threading.Thread(target=trio.run, args=(self._trio_main,), daemon=True).start()
+ self._reader_task = asyncio.create_task(self._asyncio_reader())
+ self._decay_task = asyncio.create_task(self._decay_counters())📝 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.
| threading.Thread(target=trio.run, args=(self._trio_main,), daemon=True).start() | |
| asyncio.create_task(self._asyncio_reader()) | |
| asyncio.create_task(self._decay_counters()) | |
| threading.Thread(target=trio.run, args=(self._trio_main,), daemon=True).start() | |
| self._reader_task = asyncio.create_task(self._asyncio_reader()) | |
| self._decay_task = asyncio.create_task(self._decay_counters()) |
🧰 Tools
🪛 Ruff (0.15.20)
[warning] 88-88: Store a reference to the return value of asyncio.create_task
(RUF006)
[warning] 89-89: 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 `@minichain/p2p.py` around lines 87 - 89, The background tasks started in the
p2p node setup are being created and then discarded, so `_asyncio_reader` and
`_decay_counters` may be garbage-collected while still running. Update the
`P2P`/node initialization flow to store the `asyncio.create_task(...)` results
on `self` (or another long-lived owner) and keep those references for the
lifetime of the node, alongside the existing `trio.run` thread startup.
Source: Linters/SAST tools
| # Only apply interception for content-bearing message types. | ||
| if msg_type in ("tx", "block") and status is not None: | ||
| await self._handle_validation_status(peer_id, peer_addr, status) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP -C3 'def validate_block_link_and_hash'
rg -nP -C2 '"hash" in str\(exc\)' minichain/chain.pyRepository: StabilityNexus/MiniChain
Length of output: 162
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-files | rg '^(minichain/(chain|main|p2p)\.py|.*)$' >/dev/null 2>&1 || true
printf '\n== file list ==\n'
git ls-files 'minichain/*.py' | sed -n '1,120p'
printf '\n== ast outline: minichain/chain.py ==\n'
ast-grep outline minichain/chain.py --view expanded | sed -n '1,240p'
printf '\n== ast outline: minichain/main.py ==\n'
ast-grep outline minichain/main.py --view expanded | sed -n '1,260p'
printf '\n== ast outline: minichain/p2p.py ==\n'
ast-grep outline minichain/p2p.py --view expanded | sed -n '1,260p'
printf '\n== relevant symbols ==\n'
rg -n --no-heading 'ValidationStatus|validate_block_link_and_hash|add_block|make_network_handler|_handle_validation_status|MAX_FRAME_BYTES|INVALID_THRESHOLD|FAILED_THRESHOLD' minichain/*.pyRepository: StabilityNexus/MiniChain
Length of output: 6672
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== minichain/chain.py relevant slice ==\n'
sed -n '1,220p' minichain/chain.py | cat -n
printf '\n== minichain/p2p.py relevant slice ==\n'
sed -n '150,260p' minichain/p2p.py | cat -n
printf '\n== search for block handler / add_block callers ==\n'
rg -n --no-heading 'add_block\(|validate_block_link_and_hash\(|make_network_handler|register_handler|broadcast_block|ValidationStatus\.INVALID if "hash"' minichain/*.pyRepository: StabilityNexus/MiniChain
Length of output: 16593
Block fork/sync status is misclassified as INVALID. In minichain/chain.py:133-135, add_block() returns INVALID for any ValueError whose message contains hash, and validate_block_link_and_hash() raises invalid previous hash ... for a normal link mismatch. That sends forked/ahead blocks onto the immediate-ban path in _handle_validation_status(). Split link mismatches from true hash corruption before classifying as INVALID.
🤖 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/p2p.py` around lines 236 - 238, In the p2p validation flow,
`add_block()` is treating every `ValueError` mentioning “hash” as `INVALID`,
which causes normal fork/sync cases from `validate_block_link_and_hash()`
(“invalid previous hash ...”) to be misclassified and sent down the ban path in
`_handle_validation_status()`. Update the status mapping in `minichain/chain.py`
to distinguish link/previous-hash mismatches from real hash corruption, and only
return `INVALID` for the latter; then keep `minichain/p2p.py`’s
`_handle_validation_status()` logic unchanged so forked/ahead blocks are not
immediately banned.
| if len(buffer) > MAX_FRAME_BYTES: | ||
| self._to_asyncio.put(("MALFORMED", addr)) | ||
| break |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== file list ==\n'
git ls-files 'minichain/p2p.py' 'minichain/*.py' | sed -n '1,20p'
printf '\n== outline p2p.py ==\n'
ast-grep outline minichain/p2p.py --view expanded || true
printf '\n== search key symbols ==\n'
rg -n "MAX_FRAME_BYTES|MALFORMED|chain_request|chain_response|make_network_handler|limit=" minichain/p2p.py minichain -g '!**/__pycache__/**' || true
printf '\n== relevant slices from p2p.py ==\n'
nl -ba minichain/p2p.py | sed -n '1,220p'
printf '\n--- middle ---\n'
nl -ba minichain/p2p.py | sed -n '220,420p'Repository: StabilityNexus/MiniChain
Length of output: 5694
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline minichain/p2p.py --view expanded || true
rg -n "MAX_FRAME_BYTES|MALFORMED|chain_request|chain_response|make_network_handler|limit=" minichain/p2p.py minichain -g '!**/__pycache__/**' || true
nl -ba minichain/p2p.py | sed -n '240,360p'Repository: StabilityNexus/MiniChain
Length of output: 5281
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path('minichain/p2p.py')
text = p.read_text()
for needle in ['MAX_FRAME_BYTES', 'MALFORMED', 'chain_request', 'chain_response', 'make_network_handler', 'limit=']:
print(f'\n## {needle}')
for i, line in enumerate(text.splitlines(), 1):
if needle in line:
start = max(1, i-3); end = min(len(text.splitlines()), i+3)
for j in range(start, end+1):
print(f'{j}: {text.splitlines()[j-1]}')
break
PYRepository: StabilityNexus/MiniChain
Length of output: 1561
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== search for chain request/response limits across repo ==\n'
rg -n "limit\s*=\s*500|limit\s*=\s*1000000|chain_request|chain_response|send_chain_response|make_network_handler" . -g '!**/__pycache__/**' || true
printf '\n== p2p.py around misbehavior handling ==\n'
python3 - <<'PY'
from pathlib import Path
text = Path('minichain/p2p.py').read_text().splitlines()
for start, end in [(150, 260), (260, 330)]:
print(f"\n--- lines {start}-{end} ---")
for i in range(start, min(end, len(text)) + 1):
print(f"{i}: {text[i-1]}")
PY
printf '\n== call sites for send_chain_response ==\n'
rg -n "send_chain_response\(" . -g '!**/__pycache__/**' || trueRepository: StabilityNexus/MiniChain
Length of output: 10412
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
text = Path('main.py').read_text().splitlines()
for start, end in [(145, 250), (250, 290)]:
print(f"\n--- lines {start}-{end} ---")
for i in range(start, min(end, len(text)) + 1):
print(f"{i}: {text[i-1]}")
PYRepository: StabilityNexus/MiniChain
Length of output: 8341
The 1 MB frame cap can break large chain syncs. chain_response is emitted as one JSON frame, and the full-reorg path requests up to 1,000,000 blocks, so a large catch-up response can hit the MALFORMED path, disconnect the peer, and eventually contribute to a ban. Raise the cap or chunk/bound chain_request responses.
🤖 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/p2p.py` around lines 303 - 305, The fixed MAX_FRAME_BYTES check in
p2p.py is too small for large chain sync responses and can incorrectly send
peers to the MALFORMED path. Update the framing logic around the buffer length
check in the receive loop, and either increase the limit or change the chain
sync flow so `chain_response` from `chain_request` is chunked/bounded before it
reaches the parser. Make sure the fix preserves normal framing while preventing
valid full-reorg responses from being disconnected.
| # Semantic validation: amount and fee must be non-negative integers | ||
| if not isinstance(tx.amount, int) or tx.amount < 0: | ||
| return None | ||
| fee = getattr(tx, "fee", 0) | ||
| if not isinstance(fee, int) or fee < 0: | ||
| return None | ||
| return self.apply_transaction(tx) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Redundant validation — apply_transaction already re-checks amount/fee.
The amount/fee guards here are duplicated verbatim in apply_transaction (Lines 126-130), which this method delegates to. The same checks now live in three places (validate_and_apply, validate_and_apply_with_status, apply_transaction), which invites drift if the semantic contract changes. Consider dropping the guards here and relying on apply_transaction, or extracting a single _validate_amount_fee(tx) helper shared by all three entry points.
🤖 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/state.py` around lines 94 - 100, The amount/fee checks in
validate_and_apply are duplicated in apply_transaction and the other entry
point, so remove the redundant guards here and let the shared validation path
handle them, or extract a single _validate_amount_fee(tx) helper and call it
from validate_and_apply, validate_and_apply_with_status, and apply_transaction
to keep the semantic contract in one place.
3 participants
Addressed Issues:
This PR introduces basic persistent wallet storage to ensure a node's private key and address survive across restarts, addressing the issue of ephemeral wallet generation.
Key Changes:
Keystore Persistence: The node now checks for keystore.json on startup. If found, it loads the existing key; otherwise, it generates a new one and saves it to disk.
CLI Additions: Added export_key to view the raw private key and import_key to overwrite the active wallet.
Datadir Default: Changed the --datadir argument default to .minichain to ensure persistent storage is enabled out of the box.
Security Notice: Keys are currently stored unencrypted to maintain conceptual minimality. Added a warning comment noting the future need for a "spending password" encryption layer.
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
New Features
Bug Fixes