Skip to content

Consistent error handling + exit codes in wallet balance check script (fixes #7889) - #8200

Open
freeagent0x wants to merge 2 commits into
Scottcjn:mainfrom
freeagent0x:main
Open

Consistent error handling + exit codes in wallet balance check script (fixes #7889)#8200
freeagent0x wants to merge 2 commits into
Scottcjn:mainfrom
freeagent0x:main

Conversation

@freeagent0x

Copy link
Copy Markdown

Adds scripts/check_balance.sh (consistent error handling + documented exit codes) + scripts/test_check_balance.sh (mocked HTTP tests). Fixes #7889. Wallet: 0x3030dc31a5349ddba9578ce46844a4cedc7fa02c (ETH/Base)

@github-actions

Copy link
Copy Markdown
Contributor

Welcome to RustChain! Thanks for your first pull request.

Before we review, please make sure:

  • Non-doc PRs have a BCOS-L1 or BCOS-L2 label
  • Doc-only PRs are exempt from BCOS tier labels when they only touch docs/**, *.md, or common image/PDF files
  • New code files include an SPDX license header
  • You've tested your changes against the live node

Bounty tiers: Micro (1-10 RTC) | Standard (20-50) | Major (75-100) | Critical (100-150)

A maintainer will review your PR soon. Thanks for contributing!

@github-actions github-actions Bot added BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) size/M PR: 51-200 lines labels Aug 10, 2026

@FlintLeng FlintLeng left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review: Consistent Error Handling + Exit Codes in Wallet Balance Script

Reviewed on: 2026-08-13

Summary

Fixes #7889. Adds a new scripts/check_balance.sh with documented exit codes and consistent error handling. Every failure path prints a distinct error to stderr and exits with a specific code.

Exit Code Scheme ✅

Documented exit codes are the right design for a CLI tool meant to be scripted:

Code Meaning Use Case
0 success Balance fetched and printed
1 usage error Missing/invalid wallet address
2 network error DNS/connect/timeout
3 bad response Non-200 HTTP, malformed JSON, missing field
4 wallet not found HTTP 404 from RPC

Critical promise: "The script NEVER prints a balance it did not actually receive." This is the correct fail-closed design.

Error Handling ✅

Network layer: curl failures are caught via the temp-file pattern (curl writes HTTP code to $code_file, stderr to $err_file). Connection failures → exit 2.

Response layer: HTTP 404 → exit 4 (wallet not found). HTTP non-200/404 → exit 3. Malformed JSON (jq parse error) → exit 3. Missing amount_rtc field → exit 3.

Address validation: Basic regex ^[A-Za-z0-9]{20,64}$ rejects obvious garbage. Not cryptographic validation, but appropriate for a CLI sanity check.

Test Coverage ✅

test_check_balance.sh mocks the RPC with a Python HTTP server on a test port:

  • goodwallet1234567890 → returns {"amount_rtc": "42.5"} → expects exit 0
  • missingwallet12345678 → returns 404 → expects exit 4
  • malformedwallet123456 → returns not json → expects exit 3
  • emptyfieldwallet12345 → returns {"hello": "world"} (no amount_rtc) → expects exit 3

All paths tested. The mock server is cleaned up via trap.

Minor Notes

  1. Address regex is permissive: The regex allows any 20-64 char alphanumeric string, not just RTC addresses starting with "RTC". This is fine for a sanity check, but users could pass a Solana/ETH address and get a network error (exit 2 or 3) instead of a usage error (exit 1). Consider adding a prefix check if stricter validation is desired.

  2. set -euo pipefail is correct. The script fails fast on errors and undefined variables.

Wallet: RTC019e78d600fb3131c29d7ba80aba8fe644be426e

✅ LGTM — clean, well-documented CLI tool with consistent exit codes and comprehensive test coverage.

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

Labels

BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) size/M PR: 51-200 lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Inconsistent Error Handling in Wallet Balance Check Script

2 participants