refactor: flatten directory structure to single minichain package#19
refactor: flatten directory structure to single minichain package#19SIDDHANTCOOKIE wants to merge 2 commits intoStabilityNexus:mainfrom
Conversation
WalkthroughConsolidates public API surface into the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 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 |
SIDDHANTCOOKIE
force-pushed
the
feature/flatten-structure
branch
from
592e7a2 to
59b0093
Compare
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
main.py (1)
80-92:⚠️ Potential issue | 🟠 Major
stateandchain.stateare now two different, permanently diverged objects —claim_noncereads stale data.
Blockchain()(line 81) creates its own internalState()instance. The standalonestate = State()on line 80 is never mutated by any transaction or block addition;state.get_account(address)will always return a freshly-initialized account withnonce = 0.
claim_nonceonly produces correct nonces by coincidence:max(account_nonce=0, local_nonce)reduces topending_nonce_map[address], which is kept in sync withchain.stateviasync_nonce. Ifpending_nonce_maphas no entry for an address (first call before anysync_noncefor that address), the function returns0regardless of whatchain.stateactually holds for that address's nonce.The AI summary confirms this was previously
Blockchain(state)— the two objects were the same. That sharing was lost in this PR.The fix is to remove
stateand close overchaininstead:🐛 Proposed fix
async def node_loop(): logger.info("Starting MiniChain Node with Smart Contracts") - state = State() chain = Blockchain() mempool = Mempool() pending_nonce_map = {} def claim_nonce(address): - account = state.get_account(address) + account = chain.state.get_account(address) account_nonce = account.get("nonce", 0) if account else 0 local_nonce = pending_nonce_map.get(address, account_nonce) next_nonce = max(account_nonce, local_nonce) pending_nonce_map[address] = next_nonce + 1 return next_nonce🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@main.py` around lines 80 - 92, The standalone State() instance is stale and claim_nonce reads from it; remove the unused state variable and have claim_nonce consult the blockchain's canonical state (chain.state) instead of state; update any uses of state.get_account(...) inside claim_nonce to use chain.state.get_account(...), ensure pending_nonce_map and sync_nonce continue to coordinate with chain.state, and delete the now-unused State() instantiation to avoid diverged state objects.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.gitignore:
- Around line 328-332: Update the Python ignore block in .gitignore to cover
common artifacts missing from the current entries (__pycache__/, *.py[cod],
*$py.class, *.so): add virtualenv folders (e.g., .venv/, venv/, env/),
build/dist metadata (build/, dist/, *.egg-info/, pip-wheel-metadata/), test and
cache outputs (.pytest_cache/, .mypy_cache/, .cache/, .eggs/), coverage and
report files (.coverage, htmlcov/), and packaging helpers (eggs, wheel caches);
modify the Python block around the existing patterns so these extra patterns are
included together for clearer maintenance.
In `@main.py`:
- Line 94: Replace the unused lambda parameter name to follow convention: change
the anonymous callback passed to P2PNetwork from "lambda x: None" to "lambda _:
None" so the unused argument is deliberately named as a discard and silences
ARG005; update the instantiation at the P2PNetwork call site accordingly.
---
Outside diff comments:
In `@main.py`:
- Around line 80-92: The standalone State() instance is stale and claim_nonce
reads from it; remove the unused state variable and have claim_nonce consult the
blockchain's canonical state (chain.state) instead of state; update any uses of
state.get_account(...) inside claim_nonce to use chain.state.get_account(...),
ensure pending_nonce_map and sync_nonce continue to coordinate with chain.state,
and delete the now-unused State() instantiation to avoid diverged state objects.
| # Python | ||
| __pycache__/ | ||
| *.py[cod] | ||
| *$py.class | ||
| *.so |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider expanding the Python ignore block with common missing patterns.
The four patterns added (__pycache__/, *.py[cod], *$py.class, *.so) are correct and standard. However, for a Python project of this structure (with setup.py, tests/, and likely a virtual environment), several commonly generated artifacts are not yet covered.
♻️ Proposed additions to the Python ignore block
# Python
__pycache__/
*.py[cod]
*$py.class
*.so
+
+# Distribution / packaging
+dist/
+build/
+*.egg-info/
+*.egg
+
+# Virtual environments
+venv/
+.venv/
+env/
+.env
+
+# Testing & coverage
+.pytest_cache/
+.coverage
+htmlcov/📝 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.
| # Python | |
| __pycache__/ | |
| *.py[cod] | |
| *$py.class | |
| *.so | |
| # Python | |
| __pycache__/ | |
| *.py[cod] | |
| *$py.class | |
| *.so | |
| # Distribution / packaging | |
| dist/ | |
| build/ | |
| *.egg-info/ | |
| *.egg | |
| # Virtual environments | |
| venv/ | |
| .venv/ | |
| env/ | |
| .env | |
| # Testing & coverage | |
| .pytest_cache/ | |
| .coverage | |
| htmlcov/ |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.gitignore around lines 328 - 332, Update the Python ignore block in
.gitignore to cover common artifacts missing from the current entries
(__pycache__/, *.py[cod], *$py.class, *.so): add virtualenv folders (e.g.,
.venv/, venv/, env/), build/dist metadata (build/, dist/, *.egg-info/,
pip-wheel-metadata/), test and cache outputs (.pytest_cache/, .mypy_cache/,
.cache/, .eggs/), coverage and report files (.coverage, htmlcov/), and packaging
helpers (eggs, wheel caches); modify the Python block around the existing
patterns so these extra patterns are included together for clearer maintenance.
| return next_nonce | ||
|
|
||
| network = P2PNetwork(None) | ||
| network = P2PNetwork(lambda x: None) |
There was a problem hiding this comment.
Unused lambda argument — use _ per convention.
Ruff (ARG005) flags x as unused. Replace with the conventional discard name to silence the warning and signal intent.
🔧 Proposed fix
- network = P2PNetwork(lambda x: None)
+ network = P2PNetwork(lambda _: None)📝 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.
| network = P2PNetwork(lambda x: None) | |
| network = P2PNetwork(lambda _: None) |
🧰 Tools
🪛 Ruff (0.15.1)
[warning] 94-94: Unused lambda argument: x
(ARG005)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@main.py` at line 94, Replace the unused lambda parameter name to follow
convention: change the anonymous callback passed to P2PNetwork from "lambda x:
None" to "lambda _: None" so the unused argument is deliberately named as a
discard and silences ARG005; update the instantiation at the P2PNetwork call
site accordingly.
There was a problem hiding this comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.gitignore:
- Around line 327-332: The Python ignore block currently includes __pycache__/,
*.py[cod], *$py.class, and *.so but should be expanded to cover common
virtualenvs, build artifacts and test/coverage outputs; update the .gitignore by
adding patterns for virtual environments (.venv/, venv/, env/, ENV/), build/dist
and packaging metadata (build/, dist/, *.egg-info/, pip-wheel-metadata/),
testing and coverage caches (.pytest_cache/, .coverage, coverage.xml, htmlcov/),
and tool caches (mypy cache: .mypy_cache/, pip cache if needed) so files
produced by functions/tools referenced in the repo are not accidentally
committed. Ensure the new entries are placed near the existing Python block for
clarity and do not remove the current patterns (__pycache__/, *.py[cod],
*$py.class, *.so).
1 participant
Description
Hey @Zahnentferner
This PR is a direct follow-up to the recent discussions on discord and addresses the feedback regarding the project's folder structure:
To improve minimalism and make the codebase easier for new developers to explore, this PR flattens the architecture by consolidating the single-file packages into a single cohesive
minichaindirectory.Changes Made:
consensus/,core/, [network/] and [node/] into a singleminichain/directory.tests/directory to import directly from the unifiedminichainpackage.Addressed Issues:
Resolves maintainer request on discord to flatten the directory structure.
Screenshots/Recordings:
Additional Notes:
Checklist
AI Usage Disclosure
Check one of the checkboxes below:
I have used the following AI models and tools: N/A
Summary by CodeRabbit
Refactor
Tests
Chores