Repository navigation
Refactor Coin Control - #3617
malik1004x wants to merge 22 commits into
Conversation
# Conflicts: # cw_core/lib/db/sqlite.dart
# Conflicts: # cw_core/lib/db/sqlite.dart # cw_monero/lib/monero_wallet.dart # lib/di.dart # lib/src/screens/send/widgets/send_card.dart
# Conflicts: # cw_bitcoin/lib/bitcoin_wallet.dart # cw_bitcoin/lib/electrum_balance.dart # cw_bitcoin/lib/electrum_wallet.dart # cw_core/lib/db/sqlite.dart # lib/bitcoin/cw_bitcoin.dart
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
# Conflicts: # cw_core/lib/db/sqlite.dart
| printV("MoneroFrozenCoinsStore: no coin index for $id, frozen flag cached only"); | ||
| _frozen[id] = frozen; | ||
| return; | ||
| } |
There was a problem hiding this comment.
Monero freeze can stay cache-only
High Severity
setFrozen skips wallet2 when _indexes has no entry and only writes the in-memory map. beginRefresh clears those indexes, so a freeze during or before a coin refresh never reaches Coins_setFrozen and is wiped on the next record pass.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 7032e84. Configure here.
There was a problem hiding this comment.
@cursoragent Is this possible? The UI is not accessible until the coin refresh is done.
There was a problem hiding this comment.
No. A freeze from the UI cannot land in that cache-only path and then get wiped by the refresh that unlocks the screen.
The coin list stays on the loading spinner until CoinControlBloc finishes refreshUnspents(). On Monero that is updateUnspent(), which calls beginRefresh() and then record()s every output that will appear in the list before that future completes. The freeze switch exists only on the output details screen, and that screen is opened from the loaded list. FreezeToggled does nothing unless the bloc is already CoinControlLoaded, which is emitted only after that walk.
The other updateUnspent() call is inside transaction creation. The coin-control sheet is not dismissible, so a send cannot start while it is open. If a refresh is already running when the sheet opens, this open joins that same run and keeps the spinner up until the walk fills the index map again.
The empty-index branch in setFrozen is still there, and a unit test covers an unknown key image, but nothing in this flow calls it before the refresh that produced the row the user is looking at.
| } | ||
|
|
||
| newUnspentCoins.forEach((coin) { | ||
| final coinInfoList = unspentCoinsInfo.values.where( |
There was a problem hiding this comment.
Address balances skip incremental updates
Medium Severity
updateUnspentsForAddress now only replaces the UTXO list. The old updateCoins path that incremented address.balance is gone, so a scripthash update leaves per-address balances stale until a full updateAllUnspents.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 7032e84. Configure here.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a4ea4aa. Configure here.
This branch has not been deployed
1 participant
Issue Number (if Applicable): Fixes #
Description
The Coin Control feature suffered from numerous issues in recent time stemming from desynced, duplicated state or persisting information that should have never been persisted. This PR refactors the feature to minimize duplicated state across the feature as well as remove the dependency on MobX and Hive.
UnspentInfoHive Box has been completely removed. Its only two rows that were actually required to be persistent for this feature - the coin freeze status and the coin notes - were placed in SQLite tables.CoinSelectionobject tocreateTransaction. This means there is no chance of the coin selection persisting across transaction creations (previously, the coin selection was explicitly reset when pressing "Send" on the Home page).getCoinis an expensive operation, and calling it every time the freeze status was checked could cause lag).Pull Request - Checklist
Note
High Risk
Changes UTXO selection, balance/frozen accounting, and persistence for all UTXO chains plus Decred; incorrect migration or id handling could mis-freeze coins or build wrong transactions.
Overview
Coin control is refactored so freeze state, notes, and per-send UTXO selection no longer live in the legacy Hive
UnspentCoinsInfobox or on eachUnspentinstance.Frozen outputs and user notes are stored in new SQLite tables (
FrozenCoin,CoinNote), with a one-time Hive migration that maps outputs to stable ids (hash:vout, MWEB hash-only, Monero key images). Electrum-family wallets adopt a sharedCoinControlWalletmixin:spendableCoins()applies freeze flags, optionalCoinSelection, and coin-type rules (e.g. Litecoin MWEB). Sends and fee estimates take an explicitcoinSelectionon transaction credentials instead of persisted “isSending” flags.Transaction building on Bitcoin/Litecoin/Decred now builds from pre-filtered candidate UTXOs; wallet services no longer inject the Hive box, and wallet removal clears SQLite coin-control rows. Monero keeps freezes in wallet2 via
MoneroFrozenCoinsStorewith an in-memory index cache. Related API shifts include async fee estimation and UTXO signing helpers, balance updates keyed off frozen ids, and explorer URLs for coin-control UI.Reviewed by Cursor Bugbot for commit a4ea4aa. Bugbot is set up for automated code reviews on this repo. Configure here.