Skip to content

fix: clearer CLI output and docs, key file mode, contract timeout - #171

Open
SIDDHANTCOOKIE wants to merge 1 commit into
perf/fork-work-and-incremental-state-rootfrom
fix/cli-audit-small
Open

SIDDHANTCOOKIE wants to merge 1 commit into
perf/fork-work-and-incremental-state-rootfrom
fix/cli-audit-small

Conversation

@SIDDHANTCOOKIE

Copy link
Copy Markdown
Member

Stacked on #166 (base is its branch, so the diff shows only this change; retarget to main once #166 merges).

Small fixes from a MiniChain audit of the CLI. No design changes.

CLI and docs

  • README and help documented deploy/call as [amount] [fee]. The CLI takes [amount] [gas_limit] [fee_per_gas], and gas defaults to 0, so following the docs made every deploy and call fail. Both now show the real arguments and say gas is required.
  • The README deploy example (deploy examples/counter.py 0 100) ran out of gas and created no contract. It is now deploy examples/counter.py 0 20000 0.
  • After mine, print one result line per transaction (status, gas used, error, contract address). Failed deploys and calls were previously shown as plain mined transactions.
  • balance prints full addresses, so they can be copied into call/send.
  • A second send while the nonce is already pending now says so instead of listing unrelated causes.
  • help lists ban, unban, list-banned.
  • keystore.json is created owner-only (0600), matching nodekey. The README notes it is unencrypted.
  • Default P2P/RPC ports and the key file mode moved into node_config.py.

Contract execution timeout

The 5s run-time budget started at process launch, so it also counted sandbox start-up (about 4s on Windows, where processes are spawned) and any nested sub-call. Cross-contract tests always timed out there and a few unit tests were flaky. Now:

  • the budget starts when the sandbox process reports ready (start-up has its own 60s cap);
  • time spent waiting on a sub-call is not charged to the caller, since the sub-call has its own budget (depth is still capped by MAX_CALL_DEPTH);
  • both timeouts are constants in network_config.py.

A contract that loops forever is still killed after 5s of its own running time.

Testing

Full suite on this branch: 245 passed. Before the timeout change, 2 cross-contract tests already failed on #166's branch and 3 other contract tests were flaky on Windows.
Not covered by tests: the new mine output lines, the reworded rejection message, the help rows and the 0600 keystore mode.

CLI audit fixes:
- README/help: document [amount] [gas_limit] [fee_per_gas] for deploy/call,
  replace the deploy example that ran out of gas, list ban/unban/list-banned
- print a per-transaction result after mine so failed deploys/calls are shown
- balance prints full addresses
- clearer message when a second send hits an already pending nonce
- keystore.json is created owner-only (0600)
- move default ports and key file mode into node_config

Contract execution: start the 5s run-time budget when the sandbox process
reports ready, and do not charge sub-call time to the calling contract.
Process start-up on spawn platforms (Windows) took ~4s and made cross-contract
calls and some unit tests time out.
@coderabbitai

coderabbitai Bot commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 59 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration
Commits

Reviewing files that changed from the base of the PR and between 81dfa75 and 2e6c896.

Files selected for processing: 8
  • README.md
  • main.py
  • minichain/contract.py
  • minichain/identity.py
  • minichain/network_config.py
  • minichain/node_config.py
  • minichain/p2p.py
  • minichain/rpc.py
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@SIDDHANTCOOKIE
SIDDHANTCOOKIE added this pull request to stack #172 October 25, 2026 03:06
@github-actions

Copy link
Copy Markdown

Coverage

Coverage Report
FileStmtsMissCoverMissing
minichain
   __init__.py100100% 
   block.py93199%14
   chain.py2312888%188–191, 201–202, 211–212, 215–216, 220–221, 240, 266, 288–289, 291–292, 294–295, 302–303, 305–306, 314–315, 343–344
   contract.py1773182%39–42, 77–87, 92–96, 108–114, 225–227, 251–252
   identity.py250100% 
   mempool.py65395%32–34, 56
   mpt.py300100% 
   network_config.py120100% 
   node_config.py140100% 
   p2p.py36923437%89, 92, 98, 125–143, 146–147, 150–151, 164–170, 198, 206–208, 211, 214, 221–222, 226–227, 256–257, 271–277, 290, 300, 304–306, 333–334, 347–365, 373–391, 401–417, 422–606
   persistence.py1950100% 
   pow.py430100% 
   receipt.py140100% 
   rpc.py83199%92
   serialization.py80100% 
   state.py282997%118–119, 122–124, 127–128, 135–136
   transaction.py590100% 
   validators.py90100% 
TOTAL171930782% 
Tests Skipped Failures Errors Time
245 0 💤 0 ❌ 0 🔥 19.776s ⏱️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant