Bug: /wallet/balance endpoint lacks exception handling for SQLite errors

[Bill0151]

FILE: node/rewards_implementation_rip200.py
NEW ADDITION (insert after line ~65):

# Constants for API responses
RTC_DECIMAL_PRECISION = 8
DATABASE_LOCKED_ERROR_MESSAGE = "Service unavailable due to database issues"
UNEXPECTED_DATABASE_ERROR_MESSAGE = "An unexpected database error occurred"

REASON: These constants improve code readability and maintainability for API error messages and financial precision.

FILE: node/rewards_implementation_rip200.py
FUNCTION: get_balance (line ~189 in current source)
BEFORE (existing code — shown for context, do NOT include in PR):

    @app.route('/wallet/balance', methods=['GET'])
    def get_balance():
        miner_id = request.args.get('miner_id')
        if not miner_id:
            return jsonify({"error": "miner_id required"}), 400

        with sqlite3.connect(DB_PATH) as db:
            row = db.execute(
                "SELECT amount_i64 FROM balances WHERE miner_id = ?",
                (miner_id,)
            ).fetchone()

            amount_i64 = int(row[0]) if row else 0
            return jsonify({
                "miner_id": miner_id,
                "amount_i64": amount_i64,
                "amount_rtc": amount_i64 / UNIT
            })

AFTER (your replacement — this IS the PR content):

    @app.route('/wallet/balance', methods=['GET'])
    def get_balance():
        miner_id = request.args.get('miner_id')
        if not miner_id:
            return jsonify({"error": "miner_id required"}), 400

        try:
            with sqlite3.connect(DB_PATH) as db:
                row = db.execute(
                    "SELECT amount_i64 FROM balances WHERE miner_id = ?",
                    (miner_id,)
                ).fetchone()

                amount_i64 = int(row[0]) if row else 0
                return jsonify({
                    "miner_id": miner_id,
                    "amount_i64": amount_i64,
                    "amount_rtc": round(amount_i64 / UNIT, RTC_DECIMAL_PRECISION)
                })
        except sqlite3.OperationalError as e:
            print(f"Database operational error in get_balance for miner_id {miner_id}: {e}")
            return jsonify({"error": DATABASE_LOCKED_ERROR_MESSAGE}), 503
        except sqlite3.Error as e:
            print(f"Unexpected database error in get_balance for miner_id {miner_id}: {e}")
            return jsonify({"error": UNEXPECTED_DATABASE_ERROR_MESSAGE}), 500

REASON: This change adds robust error handling for database operations, preventing unhandled 500 errors and providing specific responses for operational issues like database locks, and ensures RTC amounts are rounded for consistent precision.

REASON: This new test suite thoroughly validates the /wallet/balance endpoint, covering success cases, missing/invalid parameters, and critical database error scenarios, ensuring the robustness and correctness of the API.

[github-actions]

Welcome to RustChain! Thanks for your first pull request.

Before we review, please make sure:

• Your PR has a BCOS-L1 or BCOS-L2 label
• 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!

[Bill0151]

Hi @Scottcjn Bill here, looks like the Skein has pushed 8 files are line-ending artifacts from a Windows build environment. I'll clean that up ASAP. The intended changes are only in node/rewards_implementation_rip200.py and node/tests/test_balance_endpoint.py. Sorry, still working on the Skein!

bill0151

[Scottcjn]

Good security fix addressing #1993 — wrapping the /wallet/balance endpoint in proper exception handling is the right approach.

Before we can merge:

1. Your constant placement causes a SyntaxError — move the error message constants outside the function
2. Remove the 8 whitespace-only files (CRLF normalization changes) — keep only the files with real code changes

Once fixed: 15 RTC approved.

Tip: git diff --stat before submitting to verify only intended files are changed.

[Bill0151]

Hello again @Scottcjn — pushed the fix: constants moved outside the import block and the diff is now down to the 2 intended files only.
The 3 CI failures are pre-existing tests unrelated to this PR (test_api_miners_requires_auth, test_attest_submit_strict_fixture, test_rip201_bucket_spoof). None involve the balance endpoint or my changes.
all being well -
wallet: bill0151

[zhuzhushiwojia]

Good fix for the database error handling issue. A few observations:

Positive:

• Properly wraps database operations in try-except blocks
• Returns appropriate HTTP status codes (503 for lock, 500 for general errors)
• Uses constant definitions for error messages (good practice)

Suggestions:

1. Consider adding logging at a more verbose level (e.g., debug) for production debugging
2. The error messages could include a correlation ID for easier debugging in distributed systems
3. Consider adding a circuit breaker pattern for repeated database failures

Minor nit:

• The constants RTC_DECIMAL_PRECISION, DATABASE_LOCKED_ERROR_MESSAGE, UNEXPECTED_DATABASE_ERROR_MESSAGE could be defined alongside other constants at the top of the file, not after the jsonify function.

Overall: ✅ Approve - addresses the reported bug correctly.

[Bill0151]

@zhuzhushiwojia I can see your post is for another bounty, but wanted to thank you for taking the time to share your thoughts and let you know I'll take note of them. All feedback is welcome - especially when it's mutually beneficial for the project.