Skip to content

fix(utxo): ensure ROLLBACK on ValueError in spend_box (BUG-2)#6308

Open
crowniteto wants to merge 2 commits into
Scottcjn:mainfrom
crowniteto:fix/utxo-spend-box-rollback-style-bug2
Open

fix(utxo): ensure ROLLBACK on ValueError in spend_box (BUG-2)#6308
crowniteto wants to merge 2 commits into
Scottcjn:mainfrom
crowniteto:fix/utxo-spend-box-rollback-style-bug2

Conversation

@crowniteto
Copy link
Copy Markdown

Summary

Split follow-up from PR #6146 review feedback. Addresses BUG-2.

BUG-2: spend_box ROLLBACK leak on ValueError

Problem: spend_box() had individual ROLLBACK calls before each ValueError raise, but the except ValueError: raise path bypassed the general except Exception handler. If a future code path adds a new ValueError without the explicit ROLLBACK, the transaction would leak.

Fix: The except ValueError handler now also does ROLLBACK before re-raising. Since ROLLBACK is idempotent in SQLite (a no-op on an already-rolled-back transaction), this makes the pattern safe against future additions without any behavioral change.

Tests

3 regression tests:

  • test_spend_nonexistent_returns_none
  • test_double_spend_raises_and_releases_lock: Verifies DB is usable after ValueError
  • test_spend_box_success
3 passed in 0.21s

spend_box() had individual ROLLBACK calls before each ValueError raise,
but the 'except ValueError: raise' path bypassed the general 'except
Exception' handler. If a future code path adds a new ValueError without
the explicit ROLLBACK, the transaction would leak.

Now the 'except ValueError' handler also does ROLLBACK (idempotent in
SQLite), making the pattern safe against future additions.

3 regression tests passing.

Addresses review feedback on PR Scottcjn#6146.
@github-actions github-actions Bot added BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) tests Test suite changes size/S PR: 11-50 lines labels May 25, 2026
Copy link
Copy Markdown
Contributor

@jaxint jaxint left a comment

Choose a reason for hiding this comment

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

LGTM! Great work on this PR. 🚀

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/S PR: 11-50 lines tests Test suite changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants