Skip to content

FIX(#4182): UTXO Ledger - restore spent boxes on rollback + proper coin selection#4183

Merged
Scottcjn merged 1 commit intoScottcjn:mainfrom
508704820:fix/utxo-rollback-4182
May 10, 2026
Merged

FIX(#4182): UTXO Ledger - restore spent boxes on rollback + proper coin selection#4183
Scottcjn merged 1 commit intoScottcjn:mainfrom
508704820:fix/utxo-rollback-4182

Conversation

@508704820
Copy link
Copy Markdown
Contributor

Summary

Fixes #4182 — Two critical issues in rips/rustchain-core/ledger/utxo_ledger.py:

1. Broken rollback causes fund destruction (CRITICAL)

When UtxoSet.apply_transaction() fails during output creation, the except block returns False but does not restore the already-spent boxes. This permanently destroys funds.

Before:

except Exception as e:
    # Rollback on failure (restore spent boxes)
    # In production, this would be more sophisticated
    print(f"Transaction failed: {e}")
    return False  # spent_boxes NOT restored!

After:

except Exception as e:
    # Rollback: restore spent boxes to UTXO set
    for box in spent_boxes:
        self._boxes[box.box_id] = box
        owner = self._proposition_to_address(box.proposition_bytes)
        if owner not in self._by_address:
            self._by_address[owner] = set()
        self._by_address[owner].add(box.box_id)
    for box in spent_boxes:
        self._spent.discard(box.box_id)
    return False

2. BalanceTracker.transfer() uses all UTXOs (EFFICIENCY)

The transfer method consumed every UTXO box as input, which was inefficient, privacy-leaking, and created unnecessarily large transactions.

Fix: Greedy coin selection (smallest-first) to minimize inputs.

Testing

  • Verified the fixed file parses correctly
  • The SQLite-backed node/utxo_db.py already has correct rollback via BEGIN IMMEDIATE / ROLLBACK — this fix brings the in-memory ledger to the same standard

Wallet Address

RTC9d7caca3039130d3b26d41f7343d8f4ef4592360

…roper coin selection

Two fixes in utxo_ledger.py:

1. **Broken rollback causes fund destruction**: When apply_transaction()
   fails during output creation, the except block returned False but did
   NOT restore the already-spent boxes. This permanently destroyed funds.
   Fix: restore spent_boxes to _boxes and _by_address, remove from _spent.

2. **BalanceTracker.transfer() uses all UTXOs**: The transfer method
   consumed every UTXO box as input, which was inefficient, privacy-leaking,
   and created unnecessarily large transactions.
   Fix: greedy coin selection (smallest-first) to minimize inputs.

Note: node/utxo_db.py already has correct rollback via SQLite transactions.
This fix brings the in-memory ledger to the same safety standard.

Wallet: RTC9d7caca3039130d3b26d41f7343d8f4ef4592360
@github-actions github-actions Bot added BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) size/S PR: 11-50 lines labels May 10, 2026
Copy link
Copy Markdown

@fengqiankun6-sudo fengqiankun6-sudo left a comment

Choose a reason for hiding this comment

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

PR #4183 LGTM ✅

Critical Bug Fix: UTXO Ledger Rollback

The rollback fix in apply_transaction() addresses a critical fund destruction bug where spent boxes were not restored on transaction failure, permanently destroying user funds.

Patch assessment:

  • ✅ Correct rollback logic: restores boxes to _boxes, _by_address, and clears from _spent
  • ✅ Clean exception handling with informative log message
  • ✅ No re-entrancy or state corruption risk

Efficiency Fix: Coin Selection

The greedy smallest-first coin selection properly addresses:

  • ✅ Privacy leak (prevents exposing all UTXOs to recipient)
  • ✅ Transaction size efficiency (minimum inputs selected)
  • ✅ Change calculation corrected to use selected_total

Files changed: rips/rustchain-core/ledger/utxo_ledger.py (+33/-7)

Recommendation: MERGE — critical security fix with clean implementation.

Review by fengqiankun6-sudo

Copy link
Copy Markdown

@fengqiankun6-sudo fengqiankun6-sudo left a comment

Choose a reason for hiding this comment

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

✅ APPROVED — Critical UTXO Fund Destruction Fix

PR #4183: FIX(#4182): UTXO Ledger - restore spent boxes on rollback + proper coin selection

Security Analysis

Same critical vulnerability as #4184: Broken rollback permanently destroys UTXOs

  • Old code: `except` returns `False` but does NOT restore spent boxes
  • Result: funds permanently burned on any failed transaction

Fix Verification:

  1. ✅ Tracks `spent_boxes` during spend phase
  2. ✅ On exception: restores all spent boxes to `_boxes` dict
  3. ✅ Restores to `_by_address` index with proper owner derivation
  4. ✅ Removes from `_spent` tracking
  5. ✅ Fixes `BalanceTracker.transfer()` to use greedy coin selection (smallest-first) — efficiency + privacy improvement

Severity: Critical — permanent fund loss, DoS attack vector

Note: Both PRs #4183 and #4184 fix the same rollback bug via different approaches. PR #4183 also includes a secondary coin selection optimization.

Bounty Claim

Wallet: `RTC019e78d600fb3131c29d7ba80aba8fe644be426e`
GitHub: fengqiankun6-sudo

Reviewed by: fengqiankun6-sudo

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] UTXO Ledger: Broken rollback in UtxoSet.apply_transaction() causes fund destruction

3 participants