refactor: simplify mempool to sorted queue and fix tx removal semantics#60
Merged
Zahnentferner merged 3 commits intoStabilityNexus:mainfrom Mar 15, 2026
Conversation
Contributor
WalkthroughRefactors the mempool to a flat, timestamp-sorted transaction queue with a capped selection API; updates CLI/mining to use new mempool API and chain-based nonces; tightens receiver/amount validation in p2p/CLI; adds Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI
participant Mempool as Mempool
participant Miner as Miner
participant Chain as Chain
participant Network as Network
CLI->>Mempool: submit transaction (validate receiver/amount)
CLI->>Chain: read sender nonce (chain.state)
CLI->>Network: broadcast tx
CLI->>Miner: request mine
Miner->>Mempool: get_transactions_for_block()
Mempool-->>Miner: selected txs (sorted, capped)
Miner->>Chain: propose block(with txs)
Chain-->>Miner: block accepted
Miner->>Mempool: remove_transactions(selected txs)
Miner->>Network: broadcast new block
Network->>Chain: peers receive & validate block
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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)
📝 Coding Plan
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
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)
52-67:⚠️ Potential issue | 🟠 MajorMining can get stuck retrying stale-nonce transactions.
Line 52 takes a capped queue without state-aware filtering; if any selected tx has an outdated nonce, Line 65 rejects the block, and because Line 67 only removes on success, the same stale set can be retried indefinitely.
Suggested mitigation in this function
if chain.add_block(mined_block): logger.info("✅ Block #%d mined and added (%d txs)", mined_block.index, len(pending_txs)) mempool.remove_transactions(pending_txs) chain.state.credit_mining_reward(miner_pk) return mined_block else: logger.error("❌ Block rejected by chain") + # Drop transactions that are definitely stale against current chain nonce. + stale = [ + tx + for tx in pending_txs + if tx.nonce < chain.state.get_account(tx.sender).get("nonce", 0) + ] + if stale: + mempool.remove_transactions(stale) return None
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@main.py`:
- Around line 151-153: The _is_valid_receiver function duplicates regex logic
elsewhere; extract its regex into a shared validator utility (e.g., create or
add to a validators module) and export a single function name like
is_valid_receiver; replace the local _is_valid_receiver definition in main.py
and the duplicate in minichain/p2p.py to import and call that shared
is_valid_receiver function, and update any references to use the exported name
so both modules rely on the same implementation and regex constant.
🪄 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: 876a8b6c-6178-4d2d-ba07-46d5abee7b0c
📒 Files selected for processing (2)
main.pyminichain/p2p.py
Contributor
There was a problem hiding this comment.
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)
170-267: 🧹 Nitpick | 🔵 TrivialRefactor
cli_loopinto command handlers to reduce complexity.Line 170 still carries high branch/statement complexity; splitting each command path into focused handlers will improve readability and testability.
♻️ Minimal refactor direction
+async def _handle_send(parts, sk, pk, chain, mempool, network): + ... + +async def _handle_mine(pk, chain, mempool, network): + ... + +COMMAND_HANDLERS = { + "send": _handle_send, + "mine": _handle_mine, + ... +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@main.py` around lines 170 - 267, The cli_loop function is too complex; extract each command branch into small focused handler functions (e.g., handle_balance(chain, pk), handle_send(sk, pk, chain, mempool, network, parts), handle_mine(chain, mempool, pk, network), handle_peers(network), handle_connect(network, parts), handle_address(pk), handle_chain(chain), handle_help()) and update cli_loop to just parse input and dispatch to these handlers; ensure handlers preserve current behavior and return any control signals needed (e.g., whether to break the loop), keep async for handlers that await (send, mine, connect), and reference the existing symbols Transaction, is_valid_receiver, mine_and_process_block, chain.state, mempool.add_transaction, network.broadcast_transaction, and network.broadcast_block so logic and side-effects remain identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@main.py`:
- Around line 170-267: The cli_loop function is too complex; extract each
command branch into small focused handler functions (e.g., handle_balance(chain,
pk), handle_send(sk, pk, chain, mempool, network, parts), handle_mine(chain,
mempool, pk, network), handle_peers(network), handle_connect(network, parts),
handle_address(pk), handle_chain(chain), handle_help()) and update cli_loop to
just parse input and dispatch to these handlers; ensure handlers preserve
current behavior and return any control signals needed (e.g., whether to break
the loop), keep async for handlers that await (send, mine, connect), and
reference the existing symbols Transaction, is_valid_receiver,
mine_and_process_block, chain.state, mempool.add_transaction,
network.broadcast_transaction, and network.broadcast_block so logic and
side-effects remain identical.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 36e82b5f-d1b2-428d-bc63-9ec412793dcd
📒 Files selected for processing (3)
main.pyminichain/p2p.pyminichain/validators.py
Zahnentferner
approved these changes
Mar 15, 2026
This was referenced Mar 23, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addressed Issues:
Fixes
(sender, nonce)transaction replacement instead of hard rejection.get_transactions_for_block()read-only and enforce per-block transaction cap.chain.add_block(...).tx_idand(sender, nonce); rebuild seen-set for consistency.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
Tests