refactor(key-wallet): move spendable_utxos from wallet to account#643
refactor(key-wallet): move spendable_utxos from wallet to account#643xdustinface merged 4 commits intov0.42-devfrom
Conversation
Spendability semantics are account-type specific (e.g. identity vs standard vs coinjoin), so aggregating a single "wallet spendable UTXO set" across every account hides meaningful distinctions. Drop WalletInfoInterface::get_spendable_utxos and add ManagedCoreAccount::spendable_utxos(synced_height), forcing callers to ask per account. Updates the wallet_checker tests to query the BIP44 account directly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis pull request refactors spendable UTXO filtering by relocating the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## v0.42-dev #643 +/- ##
=============================================
+ Coverage 67.48% 67.87% +0.39%
=============================================
Files 246 318 +72
Lines 49280 67804 +18524
=============================================
+ Hits 33256 46025 +12769
- Misses 16024 21779 +5755
|
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Mempool 0-conf outputs are spendable (you can chain on them in the mempool), so Utxo::is_spendable no longer requires is_confirmed || is_instantlocked — a non-coinbase UTXO is spendable unless locked. WalletCoreBalance's "spendable" field is renamed to "confirmed" and both confirmed and unconfirmed buckets now count as spendable. spendable() becomes a helper returning their sum. Renames propagate through WalletEvent::BalanceUpdated (key-wallet-manager), OnBalanceUpdatedCallback (dash-spv-ffi), and FFIBalance mappings (key-wallet-ffi), all of which were previously plumbing balance.spendable() into fields named "confirmed". update_balance now fills the two buckets based on is_confirmed || is_instantlocked so the UI distinction between settled and mempool funds is preserved. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ZocoLini
left a comment
There was a problem hiding this comment.
Quick question, are we using the spendable_utxos method or we are only testing it??
|
we are using spendable_utxos in places to decide what inputs to use, imo though transaction builder could help there, but... it doesn't matter all that much. |
|
the thing is, I don't see any usage of it in the PR other than in tests, and I don't know how we are querying the utxos in the transaction building function we expose but definitely not using his refactored logic, I have to take a look into it, maybe worth to update it |
Summary
Two related cleanups to how key-wallet exposes spendable funds.
1.
spendable_utxosmoves from wallet to accountWalletInfoInterface::get_spendable_utxosaggregated a single "spendable UTXO set" across every account in a wallet, which is misleading — spendability rules and intent differ between standard, coinjoin, identity, and provider account types. The method is removed;ManagedCoreAccount::spendable_utxos(synced_height)replaces it, forcing callers to pick an account explicitly.2. Balance:
spendable→confirmed+unconfirmedMempool 0-conf outputs are genuinely spendable (you can chain on them in the mempool), but the old code hid them in a separate
unconfirmedbucket and excluded them fromis_spendable. Changes:Utxo::is_spendabledrops theis_confirmed || is_instantlockedcheck; a non-coinbase UTXO is spendable unless locked (coinbase still needs 100 confs).WalletCoreBalance'sspendablefield is renamed toconfirmed. Bothconfirmedandunconfirmednow count as spendable funds.spendable()becomes a helper returningconfirmed + unconfirmed.ManagedCoreAccount::update_balancesplits mature, non-locked funds intoconfirmed(is_confirmed || is_instantlocked) vsunconfirmed(mempool only) so the UI can still surface the settled-vs-mempool distinction.WalletEvent::BalanceUpdated(key-wallet-manager),OnBalanceUpdatedCallback(dash-spv-ffi), andFFIBalancemappings (key-wallet-ffi). Several FFI sites were previously plumbingbalance.spendable()into fields literally namedconfirmed— fixed to usebalance.confirmed().Breaking changes
WalletInfoInterface::get_spendable_utxosremoved.WalletCoreBalance:spendablefield renamed toconfirmed;spendable()is nowconfirmed + unconfirmedrather than just the old confirmed/mature amount.WalletEvent::BalanceUpdated.spendablerenamed toconfirmed.OnBalanceUpdatedCallbackin dash-spv-ffi: second parameter renamedspendable→confirmed(same ABI, different name).Utxo::is_spendableis now strictly more permissive: mempool 0-conf non-coinbase UTXOs returntruewhere they previously returnedfalse. Coin selection is unaffected becausecoin_selection.rsalready checksis_confirmed || is_instantlockedexplicitly viainclude_unconfirmed.Test plan
cargo check --workspace --testscargo test -p key-wallet(446 unit + all integration groups pass)cargo test -p key-wallet-manager(36 pass, event + process_block renames covered)cargo test -p key-wallet-ffi --lib(200 pass)cargo test -p dash-spv --lib(368 pass)