Skip to content

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

Open
Bill0151 wants to merge 1 commit intoScottcjn:mainfrom
Bill0151:skein/T1993-Rustchain-bug-wallet-balance-endpoint-lacks-except
Open

Bug: /wallet/balance endpoint lacks exception handling for SQLite errors#2043
Bill0151 wants to merge 1 commit intoScottcjn:mainfrom
Bill0151:skein/T1993-Rustchain-bug-wallet-balance-endpoint-lacks-except

Conversation

@Bill0151
Copy link
Copy Markdown
Contributor

@Bill0151 Bill0151 commented Apr 3, 2026

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.

@Bill0151 Bill0151 requested a review from Scottcjn as a code owner April 3, 2026 23:30
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 3, 2026

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!

@github-actions github-actions bot added BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) BCOS-L2 Beacon Certified Open Source tier BCOS-L2 (required for non-doc PRs) wallet Wallet/transfer related node Node server related tests Test suite changes size/XL PR: 500+ lines labels Apr 3, 2026
@Bill0151
Copy link
Copy Markdown
Contributor Author

Bill0151 commented Apr 3, 2026

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
Copy link
Copy Markdown
Owner

Scottcjn commented Apr 4, 2026

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 Bill0151 force-pushed the skein/T1993-Rustchain-bug-wallet-balance-endpoint-lacks-except branch from b7c4bca to c4bd19e Compare April 4, 2026 10:56
@github-actions github-actions bot added the size/L PR: 201-500 lines label Apr 4, 2026
@Bill0151
Copy link
Copy Markdown
Contributor Author

Bill0151 commented Apr 4, 2026

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

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) BCOS-L2 Beacon Certified Open Source tier BCOS-L2 (required for non-doc PRs) node Node server related size/L PR: 201-500 lines size/XL PR: 500+ lines tests Test suite changes wallet Wallet/transfer related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants