Feat/smart contracts#92
Conversation
WalkthroughMiniChain now supports transaction fees and smart contract gas metering via a new Receipt model. Blocks carry receipt Merkle roots and explicit receipts; contracts execute in gas-metered subprocesses; mining rewards include transaction fees; and users can deploy and call contracts via CLI. The custom Merkle Patricia Trie was replaced with a library. ChangesReceipt Model & Transaction Fee Infrastructure
Block & Chain Receipt Integration
Contract Execution with Gas Metering & State Management
Merkle Patricia Trie Library Integration
Smart Contracts & User-Facing Features
Tests & Persistence Updates
🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Suggested reviewers
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}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
minichain/p2p.py (1)
121-144:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReject negative fees during transaction payload validation.
feeis type-checked but never range-checked. A negative fee currently passes schema validation and can break downstream economic accounting.Suggested fix
if payload["amount"] < 0: return False + if payload["fee"] < 0: + return False🤖 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 121 - 144, The transaction payload validation currently type-checks "fee" but doesn't reject negative fees; update the validation in the payload checker (the block using required_fields/optional_fields/allowed_fields and payload) to also range-check fee by returning False when payload["fee"] < 0 (similar to the existing payload["amount"] < 0 check), ensuring negative fees are rejected before further processing.main.py (1)
171-185:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate CLI help text to include
deploy/calland fee-aware syntax.The runtime help banner still advertises only legacy commands, while Lines 247–312 add user-facing
deploy/callflows and fee parameters. This makes discoverability inconsistent in the interactive shell.Suggested patch
HELP_TEXT = """ @@ -║ send <to> <amount> - send coins ║ +║ send <to> <amount> [fee] - send coins ║ +║ deploy <filepath> [amount] [fee] - deploy ║ +║ call <contract> <payload> [amount] [fee] ║ ║ mine - mine a block ║Also applies to: 247-312
🤖 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 171 - 185, The CLI help banner in HELP_TEXT is outdated and should list the new user-facing commands and fee syntax; update the HELP_TEXT constant to include entries for "deploy <path> [--fee <amount>]" and "call <contract> <method> [--fee <amount>]" (and any related flags used by the runtime flows for deploy/call), ensuring spacing/box layout matches the existing ASCII art so the interactive shell shows the new commands that are implemented by the deploy and call flows.
🤖 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 `@examples/dex.py`:
- Around line 60-70: The code parses tokens_to_sell from parts but doesn't
validate it's strictly positive; add an explicit guard after computing
tokens_to_sell (from parts and before reading/modifying storage) to raise an
error if tokens_to_sell <= 0, so functions handling sell in examples/dex.py
(variables parts, tokens_to_sell, sender, storage) never debit the sender for
non-positive amounts.
In `@main.py`:
- Around line 253-256: The cli_loop coroutine uses blocking file I/O (with
open(filepath, "r") as f: code = f.read()) which stalls the event loop; change
it to perform file reads off the event loop (e.g., await
asyncio.to_thread(lambda: open(...).read()) or use an async file library like
aiofiles and await aiofiles.open/reader) and preserve the existing
FileNotFoundError handling around the read; update the read site in async def
cli_loop to await the non-blocking read and re-raise or log the
FileNotFoundError as before so the rest of cli_loop continues to run without
blocking.
In `@minichain/chain.py`:
- Around line 49-51: Replace the existing generic error log in the genesis load
failure handler with an exception-aware log so the stacktrace is preserved: in
the except block that currently calls logger.error("Failed to load genesis
config: %s", e) (around the genesis-loading logic in minichain/chain.py), call
logger.exception("Failed to load genesis config") instead and keep the
subsequent sys.exit(1) to preserve behavior while providing full traceback
context.
In `@minichain/contract.py`:
- Around line 16-19: The gas check in the opcode event handler currently raises
OutOfGasException when self.gas <= 0, causing an off-by-one premature halt;
change the condition in the opcode branch so that after decrementing self.gas
you only raise when self.gas < 0 (i.e., allow zero gas to complete the
configured number of opcodes) — update the check in the method handling "event
== 'opcode'" where self.gas is decremented and OutOfGasException is raised.
In `@minichain/state.py`:
- Around line 77-79: The code currently only validates tx.amount; add a semantic
check to ensure tx.fee is an integer and non-negative before any arithmetic
(similar to the existing tx.amount check). Locate the validation block that
checks "if not isinstance(tx.amount, int) or tx.amount < 0: return None" and
extend it (or add adjacent logic) to also enforce "isinstance(tx.fee, int) and
tx.fee >= 0" so that subsequent calculations involving total_cost and the sender
balance update (uses of tx.fee and total_cost) cannot be abused by negative
fees.
In `@README.md`:
- Around line 121-123: The README entries for the CLI currently document
`[gas_limit]` but the CLI parser in main.py exposes `[fee]`; update the README
lines for the `deploy` and `call` commands to use `[fee]` (or change main.py's
usage strings to `[gas_limit]` if you prefer changing the code) so the
documented arguments match the implemented usage strings in main.py; ensure the
parameter name in README exactly matches the token used in main.py's command
usage.
In `@tests/test_core.py`:
- Around line 112-113: The assertion uses a hardcoded mining reward (50);
replace the magic value with the protocol constant DEFAULT_MINING_REWARD so the
expected balance is computed as amount + DEFAULT_MINING_REWARD + fee. Update the
test in tests/test_core.py to reference DEFAULT_MINING_REWARD (import it at the
top if not already present) and use it when building the expected value passed
to self.assertEqual for self.chain.state.get_account(self.bob_pk)['balance'].
---
Outside diff comments:
In `@main.py`:
- Around line 171-185: The CLI help banner in HELP_TEXT is outdated and should
list the new user-facing commands and fee syntax; update the HELP_TEXT constant
to include entries for "deploy <path> [--fee <amount>]" and "call <contract>
<method> [--fee <amount>]" (and any related flags used by the runtime flows for
deploy/call), ensuring spacing/box layout matches the existing ASCII art so the
interactive shell shows the new commands that are implemented by the deploy and
call flows.
In `@minichain/p2p.py`:
- Around line 121-144: The transaction payload validation currently type-checks
"fee" but doesn't reject negative fees; update the validation in the payload
checker (the block using required_fields/optional_fields/allowed_fields and
payload) to also range-check fee by returning False when payload["fee"] < 0
(similar to the existing payload["amount"] < 0 check), ensuring negative fees
are rejected before further processing.
🪄 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: 21f89c03-9c2a-4859-a0f7-ce13b1ae278b
📒 Files selected for processing (23)
README.mdexamples/counter.pyexamples/dex.pyexamples/stablecoin.pymain.pyminichain/block.pyminichain/chain.pyminichain/contract.pyminichain/mempool.pyminichain/mpt.pyminichain/p2p.pyminichain/persistence.pyminichain/receipt.pyminichain/state.pyminichain/transaction.pyrequirements.txtsetup.pytests/test_contract.pytests/test_core.pytests/test_persistence.pytests/test_persistence_runtime.pytests/test_protocol_hardening.pytests/test_transaction_signing.py
💤 Files with no reviewable changes (1)
- setup.py
| parts = msg['data'].split(':') | ||
| tokens_to_sell = int(parts[1]) | ||
|
|
||
| sender = msg['sender'] | ||
| sender_tokens = storage.get(sender, 0) | ||
| if sender_tokens < tokens_to_sell: | ||
| raise Exception("Insufficient token balance") | ||
|
|
||
| # Deduct tokens from user | ||
| storage[sender] -= tokens_to_sell | ||
|
|
There was a problem hiding this comment.
Validate sell amount is strictly positive before debiting sender balance.
tokens_to_sell is parsed but never checked for <= 0. Add an explicit guard before mutating balances.
Suggested fix
parts = msg['data'].split(':')
+ if len(parts) != 2:
+ raise Exception("Invalid sell format")
tokens_to_sell = int(parts[1])
+ if tokens_to_sell <= 0:
+ raise Exception("Amount must be positive")
sender = msg['sender']
sender_tokens = storage.get(sender, 0)📝 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.
| parts = msg['data'].split(':') | |
| tokens_to_sell = int(parts[1]) | |
| sender = msg['sender'] | |
| sender_tokens = storage.get(sender, 0) | |
| if sender_tokens < tokens_to_sell: | |
| raise Exception("Insufficient token balance") | |
| # Deduct tokens from user | |
| storage[sender] -= tokens_to_sell | |
| parts = msg['data'].split(':') | |
| if len(parts) != 2: | |
| raise Exception("Invalid sell format") | |
| tokens_to_sell = int(parts[1]) | |
| if tokens_to_sell <= 0: | |
| raise Exception("Amount must be positive") | |
| sender = msg['sender'] | |
| sender_tokens = storage.get(sender, 0) | |
| if sender_tokens < tokens_to_sell: | |
| raise Exception("Insufficient token balance") | |
| # Deduct tokens from user | |
| storage[sender] -= tokens_to_sell |
🧰 Tools
🪛 Ruff (0.15.15)
[error] 60-60: Undefined name msg
(F821)
[error] 63-63: Undefined name msg
(F821)
[error] 64-64: Undefined name storage
(F821)
[warning] 66-66: Create your own exception
(TRY002)
[warning] 66-66: Avoid specifying long messages outside the exception class
(TRY003)
[error] 69-69: Undefined name storage
(F821)
🤖 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 `@examples/dex.py` around lines 60 - 70, The code parses tokens_to_sell from
parts but doesn't validate it's strictly positive; add an explicit guard after
computing tokens_to_sell (from parts and before reading/modifying storage) to
raise an error if tokens_to_sell <= 0, so functions handling sell in
examples/dex.py (variables parts, tokens_to_sell, sender, storage) never debit
the sender for non-positive amounts.
| try: | ||
| with open(filepath, "r") as f: | ||
| code = f.read() | ||
| except FileNotFoundError: |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify async functions that still perform blocking open(...)
python - <<'PY'
import ast
from pathlib import Path
for p in Path(".").rglob("*.py"):
try:
tree = ast.parse(p.read_text(encoding="utf-8"))
except Exception:
continue
for node in ast.walk(tree):
if isinstance(node, ast.AsyncFunctionDef):
for sub in ast.walk(node):
if isinstance(sub, ast.Call) and isinstance(sub.func, ast.Name) and sub.func.id == "open":
print(f"{p}:{sub.lineno} async function '{node.name}' uses open(...)")
PYRepository: StabilityNexus/MiniChain
Length of output: 121
Avoid blocking disk I/O on the cli_loop event loop path.
main.py’s async def cli_loop reads the contract with with open(filepath, "r") as f: code = f.read() (line ~254), which blocks the event loop while loading large files.
Suggested patch
+from pathlib import Path
@@
- try:
- with open(filepath, "r") as f:
- code = f.read()
+ try:
+ code = await asyncio.to_thread(Path(filepath).read_text, encoding="utf-8")
except FileNotFoundError:
print(f" File not found: {filepath}")
continue📝 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.
| try: | |
| with open(filepath, "r") as f: | |
| code = f.read() | |
| except FileNotFoundError: | |
| try: | |
| code = await asyncio.to_thread(Path(filepath).read_text, encoding="utf-8") | |
| except FileNotFoundError: | |
| print(f" File not found: {filepath}") | |
| continue |
🧰 Tools
🪛 Ruff (0.15.15)
[warning] 254-254: Async functions should not open files with blocking methods like open
(ASYNC230)
[warning] 254-254: Unnecessary mode argument
Remove mode argument
(UP015)
🤖 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 253 - 256, The cli_loop coroutine uses blocking file
I/O (with open(filepath, "r") as f: code = f.read()) which stalls the event
loop; change it to perform file reads off the event loop (e.g., await
asyncio.to_thread(lambda: open(...).read()) or use an async file library like
aiofiles and await aiofiles.open/reader) and preserve the existing
FileNotFoundError handling around the read; update the read site in async def
cli_loop to await the non-blocking read and re-raise or log the
FileNotFoundError as before so the rest of cli_loop continues to run without
blocking.
Source: Linters/SAST tools
| except Exception as e: | ||
| logger.error(f"Failed to load genesis config: {e}") | ||
| logger.error("Failed to load genesis config: %s", e) | ||
| sys.exit(1) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Use traceback logging for genesis parse failures.
At Line 50, this branch exits the process after logging. Switching to logger.exception(...) preserves stack context and makes startup-failure debugging significantly easier.
Suggested patch
- except Exception as e:
- logger.error("Failed to load genesis config: %s", e)
+ except Exception:
+ logger.exception("Failed to load genesis config")
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.
| except Exception as e: | |
| logger.error(f"Failed to load genesis config: {e}") | |
| logger.error("Failed to load genesis config: %s", e) | |
| sys.exit(1) | |
| except Exception: | |
| logger.exception("Failed to load genesis config") | |
| sys.exit(1) |
🧰 Tools
🪛 Ruff (0.15.15)
[warning] 49-49: Do not catch blind exception: Exception
(BLE001)
[warning] 50-50: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
🤖 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/chain.py` around lines 49 - 51, Replace the existing generic error
log in the genesis load failure handler with an exception-aware log so the
stacktrace is preserved: in the except block that currently calls
logger.error("Failed to load genesis config: %s", e) (around the genesis-loading
logic in minichain/chain.py), call logger.exception("Failed to load genesis
config") instead and keep the subsequent sys.exit(1) to preserve behavior while
providing full traceback context.
Source: Linters/SAST tools
| if event == 'opcode': | ||
| self.gas -= 1 | ||
| if self.gas <= 0: | ||
| raise OutOfGasException("Out of gas!") |
There was a problem hiding this comment.
Fix off-by-one in gas exhaustion condition.
At Line 18, raising on <= 0 makes execution halt one opcode earlier than the configured limit. Raise only when gas drops below zero.
🔧 Proposed fix
if event == 'opcode':
self.gas -= 1
- if self.gas <= 0:
+ if self.gas < 0:
raise OutOfGasException("Out of gas!")📝 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 event == 'opcode': | |
| self.gas -= 1 | |
| if self.gas <= 0: | |
| raise OutOfGasException("Out of gas!") | |
| if event == 'opcode': | |
| self.gas -= 1 | |
| if self.gas < 0: | |
| raise OutOfGasException("Out of gas!") |
🧰 Tools
🪛 Ruff (0.15.15)
[warning] 19-19: Avoid specifying long messages outside the exception class
(TRY003)
🤖 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/contract.py` around lines 16 - 19, The gas check in the opcode
event handler currently raises OutOfGasException when self.gas <= 0, causing an
off-by-one premature halt; change the condition in the opcode branch so that
after decrementing self.gas you only raise when self.gas < 0 (i.e., allow zero
gas to complete the configured number of opcodes) — update the check in the
method handling "event == 'opcode'" where self.gas is decremented and
OutOfGasException is raised.
| # Semantic validation: amount must be an integer and non-negative | ||
| if not isinstance(tx.amount, int) or tx.amount < 0: | ||
| return False | ||
| return None |
There was a problem hiding this comment.
Validate tx.fee as a non-negative integer before applying cost math.
Line 51/Line 93 use fee in arithmetic, but Line 78 only validates amount. A negative fee can increase sender balance at Line 96 (-= total_cost), breaking economic invariants and receipt gas accounting.
🔧 Proposed fix
def validate_and_apply(self, tx):
@@
- if not isinstance(tx.amount, int) or tx.amount < 0:
+ fee = getattr(tx, "fee", None)
+ if not isinstance(tx.amount, int) or tx.amount < 0:
+ return None
+ if not isinstance(fee, int) or fee < 0:
return None
@@
- total_cost = tx.amount + getattr(tx, 'fee', 0)
+ total_cost = tx.amount + tx.fee
@@
- total_cost = tx.amount + getattr(tx, 'fee', 0)
+ total_cost = tx.amount + tx.fee
@@
- gas_limit = getattr(tx, 'fee', 0)
+ gas_limit = tx.fee🤖 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 77 - 79, The code currently only validates
tx.amount; add a semantic check to ensure tx.fee is an integer and non-negative
before any arithmetic (similar to the existing tx.amount check). Locate the
validation block that checks "if not isinstance(tx.amount, int) or tx.amount <
0: return None" and extend it (or add adjacent logic) to also enforce
"isinstance(tx.fee, int) and tx.fee >= 0" so that subsequent calculations
involving total_cost and the sender balance update (uses of tx.fee and
total_cost) cannot be abused by negative fees.
| 1. **Deploy:** `deploy <filepath> [amount] [gas_limit]` | ||
| 2. **Call:** `call <contract_address> <payload> [amount] [gas_limit]` | ||
|
|
There was a problem hiding this comment.
Align README CLI args with implemented command names.
Lines 121–123 document [gas_limit], but the CLI parser uses [fee] (see main.py command usage strings). Please keep naming consistent to avoid failed command attempts.
Suggested patch
-1. **Deploy:** `deploy <filepath> [amount] [gas_limit]`
-2. **Call:** `call <contract_address> <payload> [amount] [gas_limit]`
+1. **Deploy:** `deploy <filepath> [amount] [fee]`
+2. **Call:** `call <contract_address> <payload> [amount] [fee]`📝 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.
| 1. **Deploy:** `deploy <filepath> [amount] [gas_limit]` | |
| 2. **Call:** `call <contract_address> <payload> [amount] [gas_limit]` | |
| 1. **Deploy:** `deploy <filepath> [amount] [fee]` | |
| 2. **Call:** `call <contract_address> <payload> [amount] [fee]` |
🤖 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 `@README.md` around lines 121 - 123, The README entries for the CLI currently
document `[gas_limit]` but the CLI parser in main.py exposes `[fee]`; update the
README lines for the `deploy` and `call` commands to use `[fee]` (or change
main.py's usage strings to `[gas_limit]` if you prefer changing the code) so the
documented arguments match the implemented usage strings in main.py; ensure the
parameter name in README exactly matches the token used in main.py's command
usage.
| # Bob was the miner. Bob gets amount(40) + mining_reward(50) + fee(5) = 95 | ||
| self.assertEqual(self.chain.state.get_account(self.bob_pk)['balance'], 95) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Use protocol constant instead of hardcoded mining reward in assertion.
At Lines 112–113, hardcoding 50 couples the test to a magic value. Build expected balance from DEFAULT_MINING_REWARD to keep this test stable across reward policy updates.
Suggested patch
- # Bob was the miner. Bob gets amount(40) + mining_reward(50) + fee(5) = 95
- self.assertEqual(self.chain.state.get_account(self.bob_pk)['balance'], 95)
+ expected = 40 + self.chain.state.DEFAULT_MINING_REWARD + 5
+ self.assertEqual(self.chain.state.get_account(self.bob_pk)['balance'], expected)📝 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.
| # Bob was the miner. Bob gets amount(40) + mining_reward(50) + fee(5) = 95 | |
| self.assertEqual(self.chain.state.get_account(self.bob_pk)['balance'], 95) | |
| expected = 40 + self.chain.state.DEFAULT_MINING_REWARD + 5 | |
| self.assertEqual(self.chain.state.get_account(self.bob_pk)['balance'], expected) |
🤖 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 `@tests/test_core.py` around lines 112 - 113, The assertion uses a hardcoded
mining reward (50); replace the magic value with the protocol constant
DEFAULT_MINING_REWARD so the expected balance is computed as amount +
DEFAULT_MINING_REWARD + fee. Update the test in tests/test_core.py to reference
DEFAULT_MINING_REWARD (import it at the top if not already present) and use it
when building the expected value passed to self.assertEqual for
self.chain.state.get_account(self.bob_pk)['balance'].
1 participant
Addressed Issues:
This PR finalizes the Smart Contracts for MiniChain by introducing end-to-end execution, sandboxing, and examples.
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
Release Notes
New Features
Documentation