feat: complete MiniChain core implementation with PoW, Ed25519, and smart contract engine#7
Merged
Zahnentferner merged 29 commits intoStabilityNexus:mainfrom Feb 21, 2026
Conversation
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Updated mining logic to include timeout and cancellation options.
Replaced print statements with logging for better monitoring and debugging.
Refactor blockchain management by introducing Blockchain class, handling block creation and state transitions.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Refactor contract execution to improve safety and clarity.
Updated the Transaction class to preserve None for data and handle additional exceptions in the verify method.
Contributor
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughAdds a complete MiniChain prototype: account-based primitives (Transaction, State, Block, Blockchain), PoW consensus, Mempool, P2P networking, sandboxed Python contracts, node orchestration and CLI packaging, plus unit tests and test bootstrap. Changes
Sequence Diagram(s)sequenceDiagram
participant Node as Node (main)
participant Mempool as Mempool
participant Miner as Miner (PoW)
participant Chain as Blockchain
participant State as State
participant Network as P2P Network
Node->>Mempool: add_transaction(tx)
Mempool-->>Node: accepted / rejected
Node->>Mempool: get_transactions_for_block()
Node->>Miner: mine_block(header)
Miner->>Miner: iterate nonces, hash header
Miner-->>Node: mined block
Node->>Chain: add_block(block)
Chain->>State: validate & apply transactions
State->>State: deploy / call / transfer
Chain-->>Node: block appended
Node->>Network: broadcast_block(block)
Network-->>Network: serialize & publish
sequenceDiagram
participant Caller as ContractMachine
participant Validator as AST Validator
participant Worker as Worker Process
participant Storage as Contract Storage
Caller->>Validator: _validate_code_ast(code)
Validator-->>Caller: valid / invalid
alt valid
Caller->>Worker: spawn process (restricted builtins, resource limits)
Worker->>Worker: execute code
Worker-->>Caller: execution result & storage
Caller->>Storage: ensure JSON-serializable
alt serializable
Storage-->>Caller: commit storage
else not serializable
Caller-->>Caller: rollback / reject
end
else invalid
Caller-->>Caller: reject execution
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related issues
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 21
🤖 Fix all issues with AI agents
In `@consensus/pow.py`:
- Around line 10-13: The docstring for calculate_hash is misleading—it's
documented as hashing a "block header" but is also used to compute transaction
IDs (see _get_tx_id); update the calculate_hash docstring to a generalized
description (e.g., "Calculates SHA256 hash of a dict payload" or "Calculates
SHA256 hash of a serialized object") that reflects its use for both blocks and
transactions, ensuring the function name calculate_hash and any callers like
_get_tx_id remain unchanged.
In `@core/chain.py`:
- Around line 15-19: The __init__ currently creates self._lock after calling
_create_genesis_block(), which is fragile because _create_genesis_block() or
methods it calls (e.g., last_block) may access self._lock and raise
AttributeError; fix by initializing self._lock (threading.RLock()) before
calling _create_genesis_block() so the lock exists for any code executed during
genesis creation, keeping the order: self.chain = [], self.state = State(),
self._lock = threading.RLock(), then self._create_genesis_block().
- Around line 58-61: The block verification is re-hashing using block.to_dict()
which includes the "hash" field and thus mismatches the mining hash; update the
verification in the add_block path (where block.hash and calculate_hash(...) are
used) to call block.to_header_dict() instead of block.to_dict() so the hash is
computed from the same header fields as mine_block and valid blocks are not
rejected.
In `@core/contract.py`:
- Around line 44-47: The guard "if not account" after calling
self.state.get_account(contract_address) is dead because get_account always
returns a default truthy dict; remove that conditional block and rely on the
existing code presence check (the subsequent "if not code" / the variable
"code") to detect non-existent contracts — locate the call
self.state.get_account and the local variable account in the contract handling
function and delete the unreachable "if not account: return False" check.
- Around line 112-116: Replace the unreliable queue.empty() check in the
contract execution path with a timed get: call queue.get(timeout=...) and catch
the Empty exception instead of returning False; in practice modify the block
surrounding the queue access (the code using queue.get() after p.join()) to try
result = queue.get(timeout=some_seconds) and except queue.Empty (or
multiprocessing.queues.Empty) log the error and return False, ensuring you
import the appropriate Empty class and preserve any p.join() semantics
(optionally join after a successful get).
- Around line 61-79: The safe_builtins allowlist in core.contract.py is missing
the int builtin (causing NameError when contracts need int conversion); add
"int": int to the safe_builtins dict (near the existing entries like "str": str,
"bool": bool) so contracts can cast to int. Also harden the AST validator
_validate_code_ast by adding "exec", "eval", and "compile" to its blocklist of
forbidden names/Calls so even if builtins change later those dangerous
constructs are rejected at AST-check time.
In `@core/state.py`:
- Around line 53-64: The public method apply_transaction currently allows
callers to bypass the tx.amount type/negativity check in validate_and_apply; to
fix this, move the semantic validation into apply_transaction (or make
apply_transaction private and keep validate_and_apply public) so all callers are
validated: add the isinstance(tx.amount, int) and tx.amount >= 0 check at the
start of apply_transaction (the method named apply_transaction) and return the
same failure shape (e.g., False) on invalid input, ensuring validate_and_apply
simply delegates to apply_transaction after any higher-level checks.
- Around line 28-43: The logger.error calls in verify_transaction_logic use
f-strings which eagerly format even when logging is disabled; change each
logger.error(...) to use %-style lazy formatting (e.g. logger.error("Error:
Invalid signature for tx from %s...", tx.sender[:8])) for the three occurrences
that reference tx.sender, sender_acc['balance']/tx.amount, and
sender_acc['nonce']/tx.nonce so the message strings are built lazily; update the
three logger.error calls inside verify_transaction_logic to pass the format
string and arguments separately.
In `@core/transaction.py`:
- Around line 9-16: The constructor currently assumes timestamp is seconds and
multiplies by 1000, so feeding it an integer-millisecond timestamp from
to_dict() inflates it 1000x; add a Transaction.from_dict(cls, d) classmethod
that reads the dict's timestamp (milliseconds), converts it to seconds
(timestamp_ms / 1000) and calls __init__ with that seconds value (or constructs
the Transaction while preserving data=None behavior), ensure to_dict still emits
integer milliseconds, and update any deserialization code paths to use
Transaction.from_dict instead of calling Transaction(...) directly; reference:
Transaction.__init__, Transaction.to_dict, Transaction.from_dict, and
hash_payload (which depends on timestamp).
In `@main.py`:
- Around line 84-90: The comment in function claim_nonce is misleading: replace
the phrase "Atomically increment for next call" on the pending_nonce_map update
with a non-misleading comment such as "Increment for next call" (or "Reserve and
increment nonce for next call") to avoid implying true atomicity; update the
comment near pending_nonce_map[address] = next_nonce + 1 in claim_nonce to
reflect that this relies on single-threaded/async behavior rather than an atomic
operation.
- Line 137: The function _run_node references the local variable claim_nonce
(defined in node_loop) causing a NameError; replace all uses of claim_nonce
inside _run_node with the passed-in closure get_next_nonce (i.e., update the
references at the three spots where claim_nonce is called — currently on lines
corresponding to the usages around nonce acquisition) so _run_node uses
get_next_nonce(...) instead of claim_nonce(...); ensure the call signature and
awaited behavior match get_next_nonce as used elsewhere.
- Around line 97-125: The closure _handle_network_data currently references
network_mempool and network_chain before they are assigned, which can raise
NameError if P2PNetwork.start() (or any network callback registration) invokes
the handler early; fix by ensuring the handler captures concrete references
after creation — either move the definition of _handle_network_data below the
assignments to network_mempool and network_chain so they are in scope when the
function is created, or change the handler to accept mempool and chain as
explicit parameters (or use functools.partial) and register a bound handler
(e.g., register _handle_network_data with network_mempool and network_chain
passed in) so _handle_network_data no longer relies on late-binding globals.
- Around line 92-95: The temporary async function handle_network_data is unused
because _handle_network_data is later wired up, and the network_state variable
is assigned but never used; remove the dead handler and the unused network_state
assignment, and instantiate P2PNetwork with the real handler
(_handle_network_data) (or pass None/placeholder if a handler must be present),
ensuring only one handler definition remains (reference: handle_network_data,
_handle_network_data, network_state, and P2PNetwork).
- Around line 40-47: The Block object never carries a miner so
getattr(mined_block, "miner", None) always yields None and rewards go to
BURN_ADDRESS; fix by adding a miner field to the Block class (add a miner
parameter to Block.__init__ and store self.miner) and ensure
mine_and_process_block sets that field when creating/mining the block (pass the
miner address into the Block constructor or assign mined_block.miner before
calling state.credit_mining_reward); alternatively, if you prefer
state-tracking, store the current miner in the State (e.g., State.current_miner)
and use that in state.credit_mining_reward instead of reading mined_block.miner.
In `@network/p2p.py`:
- Around line 52-54: The broadcast_block method can raise if block.to_dict()
throws; update broadcast_block to mirror broadcast_transaction by wrapping the
call to block.to_dict() in a try/except, catching Exception, logging the error
via logger.error with context (including the block index) and returning early;
only call self._broadcast_message("minichain-global", "block", ...) if to_dict()
succeeded.
- Around line 42-50: In broadcast_transaction, tx.sender may exist but be None,
so logger.info("Network: Broadcasting Tx from %s...", sender[:5]) can raise
TypeError; change how sender is derived and sliced by normalizing it to a string
and falling back to "<unknown>" (e.g., compute a safe_sender = (str(tx.sender)
if tx.sender is not None else "<unknown>") and then use safe_sender[:5] in the
logger call) so serialization and logging won't crash when sender is None or
shorter than 5 characters.
In `@node/mempool.py`:
- Around line 49-62: get_transactions_for_block currently clears confirmed tx
IDs from _seen_tx_ids which allows identical signed transactions to be
re-accepted later; change the logic in get_transactions_for_block (and related
methods that manage _pending_txs and _seen_tx_ids) to not remove confirmed_ids
from _seen_tx_ids immediately—keep confirmed_ids in _seen_tx_ids to reject
re-submissions at the mempool level and add a separate periodic pruning
mechanism (e.g., a prune_seen_tx_ids task) to remove old IDs after a safe
retention window to avoid unbounded growth.
- Line 36: The logging call uses an f-string which bypasses lazy evaluation;
update the logger.warning call in node/mempool.py that currently references
tx_id via an f-string (the line with logger.warning(... tx_id)) to use %-style
formatting and pass tx_id as a separate argument (e.g., "Mempool: Duplicate
transaction rejected %s", tx_id) so it matches the other log calls and enables
lazy evaluation.
In `@tests/test_contract.py`:
- Around line 2-3: Remove the unused imports by deleting the import statements
for the modules named sys and os at the top of tests/test_contract.py (the lines
importing "sys" and "os"); ensure no other references to sys or os exist in
functions or tests like any_test_function to avoid breaking code and run tests
after removal.
- Around line 115-136: The test_balance_and_nonce_updates method ends with a
TODO instead of performing the "further test calls"; add a contract call
transaction to exercise state changes and assert balances/nonces: use
Transaction(self.pk, contract_addr, <call_value>, sender_after["nonce"],
data="storage['x'] = 2") (or similar), sign with self.sk, apply via
self.state.apply_transaction, then fetch account via
self.state.get_account(self.pk) and assert the balance decreased by the call
value and nonce incremented; also verify the contract storage was updated (e.g.,
inspect returned contract state or call a state.get_storage/appropriate
accessor) to confirm storage['x'] changed from 1 to 2. Ensure you reference the
existing symbols test_balance_and_nonce_updates, Transaction,
self.state.apply_transaction, and self.state.get_account.
In `@tests/test_core.py`:
- Line 5: Remove the stale inline comment from the import statement in
tests/test_core.py so the line reads simply "from core import Transaction,
Blockchain, State"; locate the import that references the Transaction,
Blockchain, and State symbols and delete the trailing "# Removed unused imports"
note to keep the test file clean.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Contributor
Author
|
hi @Zahnentferner Check this out , implemented a base setup to kick-off the project |
SIDDHANTCOOKIE
suggested changes
Feb 16, 2026
Contributor
SIDDHANTCOOKIE
left a comment
SIDDHANTCOOKIE
left a comment
There was a problem hiding this comment.
@Zahnentferner I've reviewed as asked on discord. This is my first time reviewing so many files in a codebase altogether, so I might be mistaken in some places. Happy to be corrected if I’ve misunderstood anything. Thank you!
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@core/chain.py`:
- Around line 54-56: The chain currently creates its own State() in
Blockchain.__init__ and also mutates it in Blockchain.add_block (via
temp_state.commit), while main.py creates and applies transactions to a separate
state, causing double application and divergence; fix by choosing a single owner
of state: either remove Blockchain.self.state and have Blockchain operate on a
State instance passed in (modify Blockchain.__init__ and references in add_block
to use the external state), or change main.py to use chain.state as the single
source of truth (update mine_and_process_block and any state.apply_transaction
calls to use chain.state); also correct the log call in the index-check in
add_block (logger.warning in chain.py) so the second %s is passed
self.last_block.index + 1 instead of block.index.
In `@core/contract.py`:
- Around line 140-170: The AST validator in _validate_code_ast cannot catch
runtime-constructed dunder strings (e.g., via str joins or concatenation) so
update the function's docstring and/or surrounding documentation to explicitly
state this limitation: note that ast.Constant and ast.JoinedStr checks only
catch literal "__" or f-strings, that constructs built at runtime (e.g., using
str(), join, concatenation or chr()) can bypass the static check, and that the
true security relies on the __builtins__ restriction (safe_builtins) and process
isolation; also add a clear log message/warning in _validate_code_ast indicating
this limitation for audit/debugging purposes.
- Around line 96-138: The current contract execution creates a new
multiprocessing.Process and multiprocessing.Queue for every call (see
_safe_exec_worker, p.start(), p.join(), queue.get()), which is expensive;
replace that per-call spawning with a long-lived worker pool (e.g.,
multiprocessing.Pool or concurrent.futures.ProcessPoolExecutor) so tasks are
submitted to the pool with a timeout and results retrieved from futures instead
of creating new Process/Queue each time; ensure you adapt the call site to
submit (code, globals_for_exec, context) to the pool, handle timeouts and
exceptions equivalently (log using logger.error and return False on
timeout/crash), and keep the JSON serializability check and the
state.update_contract_storage(contract_address, result["storage"]) commit
behavior unchanged.
- Around line 8-29: Summary: Ensure consistent, Windows-safe multiprocessing
behavior for the worker by enforcing spawn start method or documenting a proper
main-guard. Update code that starts processes which call _safe_exec_worker to
either (A) call multiprocessing.set_start_method("spawn", force=True) early at
module import (e.g., top of this module) so child processes use spawn
consistently across platforms, or (B) ensure all process creation that targets
_safe_exec_worker is performed under an if __name__ == "__main__" guard in the
caller module; reference _safe_exec_worker and any call sites that create
multiprocessing.Process objects and pass result_queue so reviewers can locate
the relevant process-start logic. Ensure the chosen approach is applied once
(not repeatedly) and document the decision in a short comment near the
process-launch code.
In `@main.py`:
- Around line 141-258: The _run_node function currently contains a hardcoded
demo flow (creating wallets via create_wallet, crediting via
state.credit_mining_reward, building Transactions, broadcasting via
network.broadcast_transaction, and mining via mine_and_process_block) which
makes it non-reusable; refactor by extracting the linear demo logic into a
separate function (e.g., run_demo or demo_scenario) that contains the calls to
create_wallet, Transaction construction/signing, mempool.add_transaction,
network.broadcast_transaction, and mine_and_process_block, and leave _run_node
as the reusable node entrypoint that only starts the network (network.start),
wires components (state, chain, mempool, pending_nonce_map, get_next_nonce), and
accepts an optional callback/flag to run the demo; ensure no demo-specific
hardcoded wallets or state mutations remain in _run_node and instead pass any
demo dependencies into the new demo function as parameters.
- Around line 80-101: node_loop instantiates a standalone State() and a
Blockchain() that creates its own State, causing a dual-state bug; fix by
ensuring the Blockchain uses the same State instance: modify how Blockchain is
constructed or its attribute is set so that the Blockchain created in node_loop
uses the existing state (reference symbols: node_loop, State, Blockchain, chain,
state); either change Blockchain's constructor to accept a State and pass state
into Blockchain(state) or, if constructor cannot change, immediately assign
chain.state = state right after chain = Blockchain() so all subsequent ops
(genesis credit, nonce sync, contract storage reads, and block validation)
operate on the same State instance.
- Around line 24-69: The bug is that mine_and_process_block applies transactions
twice using two different State instances: chain.add_block(mined_block) applies
to chain.state, while the function then re-applies each tx to the caller's state
via state.apply_transaction(tx), causing rejections and double application; fix
by consolidating on a single State instance — either remove the separate state
usage in mine_and_process_block and use chain.state for crediting mining
rewards, nonce syncs and applying/inspecting tx results, or refactor
Blockchain.__init__ to accept the node's state and ensure chain.state is that
same State, then remove the extra state.apply_transaction calls and any code
paths that assume a different state (references: mine_and_process_block,
chain.add_block, state.apply_transaction, Blockchain.__init__, and any
nonce-syncing helpers like sync_nonce).
---
Duplicate comments:
In `@core/contract.py`:
- Around line 45-47: The guard checking falsiness of account is dead because
self.state.get_account(contract_address) always returns a truthy dict; remove
the unreachable check (the "if not account: return False" block) from the method
that calls self.state.get_account(contract_address) so the code no longer
contains a redundant early return and relies on the initialized account object
instead; if a semantic change was intended instead, adjust State.get_account to
return None on missing accounts and update callers accordingly (references:
self.state.get_account and the surrounding method in core/contract.py).
- Around line 61-79: The safe_builtins allowlist in safe_builtins is missing int
which causes NameError when contracts call int(); add int to the safe_builtins
mapping alongside other primitives (e.g., "int": int) and update any related
tests; additionally, add "exec", "eval", and "compile" to the AST
blocklist/forbidden-names check used by the contract AST validation (where AST
checks are implemented) to ensure defense-in-depth against dynamic execution.
In `@main.py`:
- Around line 36-52: The code currently unconditionally sets mined_block.miner =
BURN_ADDRESS which causes the later validation (getattr + re.match) to always
treat the miner as the burn address; remove the unconditional assignment and
instead use the existing getattr/validation workflow to determine miner_address,
assigning BURN_ADDRESS only if the miner attribute is missing or invalid; update
references around mined_block, miner, BURN_ADDRESS, getattr(mined_block,
"miner", None), re.match(...), and state.credit_mining_reward(miner_address) so
rewards go to the real miner when present.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@core/block.py`:
- Line 4: Replace deprecated typing generics in core/block.py: remove List and
Optional from the import line and update all type annotations to use built-in
generics (e.g., change typing.List[Transaction] → list[Transaction] and
typing.Optional[...] → ... | None). Update annotations in the Block class, its
__init__ signature, any methods referencing transactions or optional fields
(e.g., transaction lists, previous_hash, height, timestamp), and any
variable/return type hints that use List or Optional so they use list[...] and X
| None instead.
- Around line 42-48: The constructor for Block accepts None for transactions at
runtime but is typed as List[Transaction>; update the signature or the
implementation to match: change the parameter type in Block.__init__ from
transactions: List[Transaction] to transactions: Optional[List[Transaction]] and
keep the existing fallback self.transactions = transactions or [] (or
alternatively remove the fallback and require callers to pass a list); ensure
the import/annotation for Optional is present and that the attribute annotation
self.transactions stays List[Transaction] after normalizing the input.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@core/block.py`:
- Around line 12-34: _calculate_merkle_root currently hashes the entire
tx.to_dict() (including the signature) which couples the Merkle root to signing
state; change it to hash only the transaction payload (exclude signature) by
calling the transaction payload-hash helper (e.g., use
Transaction.hash_payload() or the module-level hash_payload(tx) function) when
constructing tx_hashes so the Merkle root reflects only transaction content and
is stable before signing; update references in _calculate_merkle_root
accordingly (replace tx.to_dict() usage with the payload-only hash helper).
- Around line 50-55: The Block constructor currently always multiplies the
provided timestamp by 1000, causing already-millisecond timestamps to be
inflated; update the Block.__init__ logic that sets self.timestamp to detect and
accept millisecond timestamps as-is (e.g., if timestamp is not None then: if
timestamp > 1e11 or timestamp > time.time() * 1000 // 10 treat as already-ms and
set self.timestamp = round(timestamp) else set self.timestamp = round(timestamp
* 1000)); reference the Block.__init__ assignment to self.timestamp and adjust
accordingly (or alternatively add a Block.from_dict deserializer that assumes
ms) so reconstructed blocks keep correct millisecond timestamps and hash
verification succeeds.
In `@main.py`:
- Line 15: The BURN_ADDRESS constant and the public-key validation expect 40-hex
characters but Ed25519 public keys are 32 bytes (64 hex chars), causing valid
miner keys to fail and default to the burn address; update BURN_ADDRESS to a
64-hex zero string and change the validation/regex that currently checks for
exactly 40 hex chars to check for 64 hex chars (and update any related checks in
the same block/range referenced as 40-53) so real 64-char PyNaCl public keys
pass validation (adjust symbols: BURN_ADDRESS and the regex/validation function
that enforces the hex length).
- Around line 58-65: The current heuristic uses isinstance(result, dict) (from
chain.state.get_account(tx.receiver)) which treats any account as a contract;
instead detect contracts by checking a contract-specific field (e.g., ensure
result is a dict and has a non-empty "code" key) or call a dedicated helper if
available (e.g., chain.state.is_contract or similar) before appending to
deployed_contracts; update the check around get_account(tx.receiver) /
deployed_contracts accordingly.
- Line 142: The variable bob_sk returned from create_wallet() is unused; rename
it to _bob_sk (or _bob_sk) to signal intentional unused value and avoid linter
warnings. Update the assignment to _bob_sk, bob_pk = create_wallet() wherever
create_wallet() is used here, and ensure no other references to bob_sk remain
(use bob_pk where needed).
- Line 138: The _run_node function currently declares a unused state parameter;
remove the extra parameter from the async def _run_node signature and from every
call site that invokes _run_node so the function uses chain.state consistently
(leave references to chain.state as-is). Search for the symbol _run_node and
update its signature and all invocations to drop the state argument so parameter
lists match.
---
Duplicate comments:
In `@core/block.py`:
- Line 4: The import and type hints use typing.List which is deprecated; remove
List from the import (keep Optional if needed) and replace all occurrences of
typing.List or List[...] type hints in this file with the built-in generic
list[...] syntax (e.g., change List[Foo] to list[Foo]); update the import line
that currently references List so it only imports Optional (or remove typing
import entirely if no longer needed) and ensure any annotations in
classes/functions in this module that referenced List (search for occurrences of
"List[") are updated to use "list[" instead.
In `@main.py`:
- Around line 97-130: P2PNetwork is instantiated without a handler (network =
P2PNetwork(None)) and its handler_callback is set later, creating a race where
internal setup or callbacks in P2PNetwork.__init__ could invoke a None handler;
fix by passing the handler to the constructor (e.g., call
P2PNetwork(_handle_network_data) or modify P2PNetwork to accept a default no-op)
or ensure P2PNetwork.__init__ does not invoke the handler before assignment;
update the instantiation and/or P2PNetwork signature so _handle_network_data is
provided at construction rather than assigning network.handler_callback after
creating network.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Zahnentferner
approved these changes
Feb 21, 2026
This was referenced Feb 21, 2026
Closed
aniket866
changed the title
This was referenced Mar 11, 2026
11 tasks
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
3 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:
Closes #1
1. Overview
This Pull Request delivers the fully functional core implementation of MiniChain.
Key Objectives Achieved
2. What This PR Implements
2.1 Core Blockchain Features
2.2 Smart Contract System (Bonus Requirement)
2.3 Architecture Refactor
2.4 Type of Change
3. Project Folder Structure
3.1 Module Responsibilities
Core Layer
Consensus Layer
Network Layer
Node Layer
Testing
4. Testing Instructions
Step 1 — Setup Environment
Install dependencies:
Step 2 — Run Node Simulation
Executes full blockchain lifecycle:
Genesis → Transfer → Contract Deployment → Contract Execution
Step 3 — Run Unit Tests
Validate core and smart contracts:
Expected Test Output
5. Technical Highlights
6. Why This PR Matters
Screenshots of successful execution and testing
Checklist
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.
Summary by CodeRabbit
New Features
Tests
Chores