[UTXO-BUG] Preserve caller-owned UTXO DB connection by iamdinhthuan · Pull Request #5572 · Scottcjn/Rustchain

iamdinhthuan · 2026-05-17T16:00:14Z

Summary

stop UtxoDB.apply_transaction() from closing caller-supplied SQLite connections on the rejected mining_reward guard path
add a regression proving a caller can still inspect and roll back its external transaction after a rejected mint attempt

Why

apply_transaction(..., conn=caller_conn) accepts a caller-owned connection. On the unauthorized mining_reward validation path, the current code closes that external connection before returning False. That turns a normal validation failure into sqlite3.ProgrammingError: Cannot operate on a closed database for callers that need to inspect or roll back their own transaction.

Validation

# RED before production fix
python3 -m pytest node/test_utxo_db.py::TestUtxoDB::test_user_supplied_mining_reward_preserves_external_conn node/test_utxo_db.py::TestUtxoDB::test_user_supplied_mining_reward_rejected_before_opening_db -q

Observed before fix: test_user_supplied_mining_reward_preserves_external_conn failed because the caller-owned connection was closed.

python3 -m pytest node/test_utxo_db.py::TestUtxoDB::test_user_supplied_mining_reward_preserves_external_conn node/test_utxo_db.py::TestUtxoDB::test_user_supplied_mining_reward_rejected_before_opening_db node/test_utxo_db.py -q
python3 -m py_compile node/utxo_db.py node/test_utxo_db.py
git diff --check

Observed locally: 72 passed, py_compile passed, and git diff --check was clean.

Duplicate / safety checks

Before opening this PR I searched public issues and PRs for caller-owned connection apply_transaction mining_reward, external conn close apply_transaction mining_reward, and preserve external connection apply_transaction. I did not find an exact duplicate. Open PR #4638 is about unmineable mempool outputs, not this external-connection ownership path.

Bounty route: Scottcjn/rustchain-bounties#2819.
Miner / payout id: iamdinhthuan.
No private keys, tokens, credentials, or account-internal data are included.

github-actions · 2026-05-17T16:00:22Z

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!

ZacharyZhang-NY

Reviewed PR #5572 at commit 6123eb4. The change correctly preserves caller-owned SQLite connection ownership on the rejected mining_reward guard path while keeping owned connections unopened for unauthorized mint attempts. Validation passed: git diff --check for node/utxo_db.py and node/test_utxo_db.py; py_compile for both files; focused UTXO connection-ownership tests passed with 2 passed; full node/test_utxo_db.py passed with 72 passed. I did not find a blocking issue in this patch.

LubuSeb

Disclosure: I am reviewing this under Scottcjn/rustchain-bounties#73 as @LubuSeb.

Approved on head 6123eb4722b36fecdcc0894219b4d47239d7c525.

The change is narrowly scoped and fixes the ownership bug without weakening the unauthorized mining_reward guard. apply_transaction() now rejects user-supplied mint attempts before opening an owned DB connection, while preserving a caller-supplied connection for callers that need to inspect or roll back their own transaction. The added regression covers the external-connection path, and the existing UTXO tests still pass.

Validation I ran:

git diff --check origin/main...HEAD -- node/utxo_db.py node/test_utxo_db.py
python -B -m py_compile node/utxo_db.py node/test_utxo_db.py
python -B -m pytest -q node/test_utxo_db.py::TestUtxoDB::test_user_supplied_mining_reward_preserves_external_conn node/test_utxo_db.py::TestUtxoDB::test_user_supplied_mining_reward_rejected_before_opening_db node/test_utxo_db.py --tb=short
# 72 passed

I did not find a blocking issue in this PR.

TJCurnutte

Approved. I validated this head locally and the ownership boundary is now right.

Proof run at head 6123eb4722b36fecdcc0894219b4d47239d7c525:

git diff --check origin/main...HEAD -- node/test_utxo_db.py node/utxo_db.py
PYTHONPATH=node python3 -B -m py_compile node/utxo_db.py node/test_utxo_db.py
PYTHONPATH=node python3 -B -m pytest -q -o addopts='' --noconftest node/test_utxo_db.py

Result: 72 passed in 0.38s.

I also ran a direct caller-owned-connection probe against the rejected mining_reward path. apply_transaction() returned False, SELECT 1 on the same connection still worked, and conn.in_transaction stayed True, so the caller still owns cleanup/rollback after the rejection.

The two-line code change is scoped: the unauthorized minting guard still rejects user-supplied mining_reward, but it no longer closes a connection it did not create. The new regression test covers that exact failure mode.

508704820 · 2026-05-17T16:26:20Z

Security Review ✅ HIGH

Preserve caller-owned UTXO DB connection on rejected mining reward path. Previously, apply_transaction() would close the caller-supplied connection on rejection, preventing the caller from rolling back its external transaction. This could leave the database in an inconsistent state — a state corruption bug.

Reviewed by Xeophon - Solana: Lt9nERv6VHsojw15LpFeiaabuphAggzfLF9sM9UXRrZ

weilixiong · 2026-05-17T16:40:28Z

Claiming and fixing - one-line fix: change "if conn:" to "if own:" on mining_reward guard path.

weilixiong · 2026-05-17T16:40:51Z

Claiming and fixing - one-line fix: change "if conn:" to "if own:" on mining_reward guard path.

HCIE2054

LGTM!

jaxint

LGTM! Great work on this PR. 🚀

HCIE2054

LGTM!

kekehanshujun

I reviewed the UTXO connection ownership fix. The previous early-return path closed conn even when the caller supplied it, which breaks callers that are managing an outer transaction. Removing that close and adding a regression that keeps the externally-owned connection usable after a rejected unauthorized mining_reward addresses the issue directly.

I do not see a blocker in this focused patch.

HCIE2054

LGTM!

HCIE2054

LGTM!

HCIE2054

LGTM!

HCIE2054

LGTM!

HCIE2054

LGTM!

HCIE2054

LGTM!

HCIE2054

LGTM!

HCIE2054

LGTM!

HCIE2054

LGTM!

fix: preserve caller-owned UTXO DB connection

6123eb4

github-actions Bot added BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) node Node server related labels May 17, 2026

github-actions Bot added the size/S PR: 11-50 lines label May 17, 2026

ZacharyZhang-NY approved these changes May 17, 2026

View reviewed changes

ZacharyZhang-NY mentioned this pull request May 17, 2026

[Bounty Claim] PR Review - RustChain PR #5572 (Bounty #73) Scottcjn/rustchain-bounties#10632

Open

LubuSeb approved these changes May 17, 2026

View reviewed changes

ZacharyZhang-NY mentioned this pull request May 17, 2026

[Bounty Claim Consolidation] ZacharyZhang-NY PR Reviews for Bounty #73 Scottcjn/rustchain-bounties#10608

Open

LubuSeb mentioned this pull request May 17, 2026

Claim: Standard PR review for RustChain #5572 Scottcjn/rustchain-bounties#10633

Open

TJCurnutte approved these changes May 17, 2026

View reviewed changes

TJCurnutte mentioned this pull request May 17, 2026

[BOUNTY] Code Review Bounty Program — Review PRs, Earn RTC (100 RTC Pool) Scottcjn/rustchain-bounties#73

Open

weilixiong mentioned this pull request May 17, 2026

fix(#5572): preserve caller-owned SQLite connection on rejected mining_reward #5578

Closed

HCIE2054 approved these changes May 17, 2026

View reviewed changes

jaxint approved these changes May 17, 2026

View reviewed changes

jaxint mentioned this pull request May 17, 2026

PR Review Claim - 40 PRs - 2026-05-17 18:08 UTC Scottcjn/rustchain-bounties#10712

Open

HCIE2054 approved these changes May 17, 2026

View reviewed changes

kekehanshujun approved these changes May 17, 2026

View reviewed changes

kekehanshujun mentioned this pull request May 17, 2026

[Bounty Claim] RustChain review batch - UTXO, withdrawal tests, SDK defaults, Windows diagnostics Scottcjn/rustchain-bounties#10754

Open

HCIE2054 approved these changes May 17, 2026

View reviewed changes

HCIE2054 mentioned this pull request May 17, 2026

PR Review Batch - 100 PRs - RTC Scottcjn/rustchain-bounties#10784

Open

HCIE2054 approved these changes May 17, 2026

View reviewed changes

HCIE2054 approved these changes May 18, 2026

View reviewed changes

Scottcjn merged commit 9e219f5 into Scottcjn:main May 18, 2026
11 of 12 checks passed

Uh oh!

Conversation

iamdinhthuan commented May 17, 2026

Summary

Why

Validation

Duplicate / safety checks

Uh oh!

github-actions Bot commented May 17, 2026

Uh oh!

ZacharyZhang-NY left a comment

Choose a reason for hiding this comment

Uh oh!

LubuSeb left a comment

Choose a reason for hiding this comment

Uh oh!

TJCurnutte left a comment

Choose a reason for hiding this comment

Uh oh!

508704820 commented May 17, 2026

Uh oh!

weilixiong commented May 17, 2026

Uh oh!

weilixiong commented May 17, 2026

Uh oh!

HCIE2054 left a comment

Choose a reason for hiding this comment

Uh oh!

jaxint left a comment

Choose a reason for hiding this comment

Uh oh!

HCIE2054 left a comment

Choose a reason for hiding this comment

Uh oh!

kekehanshujun left a comment

Choose a reason for hiding this comment

Uh oh!

HCIE2054 left a comment

Choose a reason for hiding this comment

Uh oh!

HCIE2054 left a comment

Choose a reason for hiding this comment

Uh oh!

HCIE2054 left a comment

Choose a reason for hiding this comment

Uh oh!

HCIE2054 left a comment

Choose a reason for hiding this comment

Uh oh!

HCIE2054 left a comment

Choose a reason for hiding this comment

Uh oh!

HCIE2054 left a comment

Choose a reason for hiding this comment

Uh oh!

HCIE2054 left a comment

Choose a reason for hiding this comment

Uh oh!

HCIE2054 left a comment

Choose a reason for hiding this comment

Uh oh!

HCIE2054 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants