Feat/smart contracts#92
Conversation
WalkthroughMiniChain adds fee-bearing transactions, a Receipt model, gas-metered contract execution in subprocesses, receipt Merkle roots and payloads inside blocks, MPT replaced by trie.HexaryTrie, CLI deploy/call commands, example contracts, and tests updated for receipt/gas semantics. 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 | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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'].
| def _safe_exec_worker(code, globals_dict, context_dict, result_queue, gas_limit): | ||
| """ | ||
| Worker function to execute contract code in a separate process. | ||
| Worker function to execute contract code in a separate process with gas metering. |
There was a problem hiding this comment.
Besides running the contract code in a separate process and with resource limits, are we doing anything to prevent malicious code from being executed?
For example, if I write code that will make the node delete some critical files, what will prevent this code from being executed? Another example: if I write code that leads the node to call an external API, what will prevent this?
Could you document this better in the code itself?
There was a problem hiding this comment.
Sure, I have updated minichain/contract.py to explicitly document the Sandboxing enforcement mechanisms directly in the docstrings for _safe_exec_worker and the ContractMachine class. I explained how we completely strip the builtins dictionary to block open(), import, eval(), etc., which inherently prevents file IO and OS-level commands. I also listed all the explicitly allowed builtins there as requested.
| # This is a simple counter contract designed to demonstrate the basic | ||
| # structure of smart contracts in MiniChain. | ||
| # | ||
| # Available built-ins in the MiniChain Sandbox: |
There was a problem hiding this comment.
In addition to documenting this here, could you also document the available built-ins in a more suitable location (e.g. in the place where these built-ins are being made available)?
| # | ||
| # Available functions: range(), len(), min(), max(), abs(), str(), bool(), float(), int(), list(), dict(), tuple(), sum() | ||
| # | ||
| # NOTE: The sandbox does NOT allow imports, print(), or any double-underscore methods. |
There was a problem hiding this comment.
Could you also document this in the place where this is being enforced?
| # | ||
| # NOTE: The sandbox does NOT allow imports, print(), or any double-underscore methods. | ||
|
|
||
| if msg['data'] == 'increment': |
There was a problem hiding this comment.
Interesting! this is good enough for MiniChain. For a production-ready chain, it would be interesting to find a way to allow contracts devs to write non-embedded python code, such as:
def increment():
current_value = counter
counter = current_value + 1
Instead of having to use the msg and storage built-ins explicitly.
Let's discuss our options in discord.
There was a problem hiding this comment.
Regarding non-embedded Python code for contracts (like defining actual functions instead of just if msg['data'] == ...), we could solve this by having the deployment transaction save the raw Python functions into state. Then, when a user makes a contract call, the payload could literally just be increment() or transfer('0x...', 50). Our sandbox could exec() the contract code to load the functions into memory, and then eval() the user's payload to trigger the specific function. We would just need to carefully validate the AST of the payload to ensure they only call defined functions. Can we proceed with this?
| def trace_calls(self, frame, event, arg): | ||
| frame.f_trace_opcodes = True | ||
| if event == 'opcode': | ||
| self.gas -= 1 |
There was a problem hiding this comment.
There things we could do to improve how fees and gas limits are interacting. Could you add an issue for us to discuss this later?
There was a problem hiding this comment.
Out of curiosity: are we able to easily know how much storage a contract is using?
There was a problem hiding this comment.
Yes, Since the contract storage is just a Python dictionary that gets serialized, we can easily calculate its size at any time by measuring the byte length of its JSON string: len(json.dumps(account['storage'])). We could eventually use this to charge 'Storage Rent' or exact gas fees for writing data.
There was a problem hiding this comment.
Yes. This is what I was thinking. Could you add an issue int he issue tracker for this?
| self.assertEqual(sender_after["balance"], initial_balance - 10) | ||
| self.assertEqual(sender_after["balance"], initial_balance - 510) | ||
| self.assertEqual(sender_after["nonce"], initial_nonce + 1) | ||
|
|
There was a problem hiding this comment.
Could you also add some tests to show that those malicious examples that I mentioned in another comment lead to contract execution failure?
Also, could you add some tests showing that trying to use functions and imports that are not allowed in the sandbox leads to failure?
There was a problem hiding this comment.
Done! I added test_malicious_import and test_malicious_file_deletion to tests/test_contract.py. They attempt to import the os module and use open() to write to a file. Both tests confirm that the transactions fail gracefully and throw the correct AST Validation Failed and NameError exceptions.
| storage['native_reserve'] = new_native_reserve | ||
| storage['token_reserve'] = new_token_reserve | ||
|
|
||
| # Wait! In MiniChain, smart contracts cannot arbitrarily initiate outgoing transactions yet. |
There was a problem hiding this comment.
Could you add an issue in our issue tracker for this?
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@minichain/contract.py`:
- Around line 78-79: The docstring advertises int() as allowed but the sandbox's
allowlist omits it; update the safe_builtins mapping(s) in minichain/contract.py
to include 'int' (pointing to the built-in int) so contracts relying on int()
don't raise NameError — locate the safe_builtins variable(s) referenced in the
Contract class/module and add the 'int': int entry in both places mentioned (the
block around the Allowed Builtins comment and the subsequent safe_builtins
definition at lines ~117-124).
🪄 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: 61b19aee-5e98-4470-b924-a4546407cff6
📒 Files selected for processing (2)
minichain/contract.pytests/test_contract.py
| Allowed Builtins: range(), len(), min(), max(), abs(), str(), bool(), float(), int(), list(), dict(), tuple(), sum(), Exception | ||
| Blocked Builtins: Imports, File IO (open), OS modules, Networking, Introspection. |
There was a problem hiding this comment.
int() is documented as allowed but never exposed.
The class docstring advertises int() in the sandbox API, but safe_builtins omits it. Contracts written against the documented allowlist will currently fail with NameError.
Also applies to: 117-124
🤖 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 78 - 79, The docstring advertises int()
as allowed but the sandbox's allowlist omits it; update the safe_builtins
mapping(s) in minichain/contract.py to include 'int' (pointing to the built-in
int) so contracts relying on int() don't raise NameError — locate the
safe_builtins variable(s) referenced in the Contract class/module and add the
'int': int entry in both places mentioned (the block around the Allowed Builtins
comment and the subsequent safe_builtins definition at lines ~117-124).
| def trace_calls(self, frame, event, arg): | ||
| frame.f_trace_opcodes = True | ||
| if event == 'opcode': | ||
| self.gas -= 1 |
There was a problem hiding this comment.
Yes. This is what I was thinking. Could you add an issue int he issue tracker for this?
2 participants
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
New Features
Documentation
Tests