Wallet: Compute GUI balances in one wallet scan to fix per-block stutter - #1896
Wallet: Compute GUI balances in one wallet scan to fix per-block stutter#1896reubenyap wants to merge 1 commit into
Conversation
The 250 ms balance poll recomputes balances on the GUI thread whenever the block height changes. TryGetBalances performed four full scans of mapWallet (available, unconfirmed, immature and mintable), calling IsTrusted up to three times per transaction. The mintable scan, GetBalance(true), was the worst offender: the exclude-locked variant of GetAvailableCredit can never be served from the per-transaction credit cache, and it also overwrote and invalidated that cache on every call, forcing the next poll's available scan to recompute every credit from scratch. On large wallets this shows up as GUI stutter pulsing with block arrivals. Fold the four sums into a single scan that calls IsTrusted once per transaction, reuse the available credit for the mintable sum when no coins are locked (excluding locked coins only skips outputs in setLockedCoins, so the values are identical), and stop the exclude-locked credit computation from touching the cache it never reads. Per-transaction conditions and credit calls are otherwise unchanged, so the computed amounts stay identical. Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01TV7dpUCcTQrSSxnXaR7Xuq
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Summary by CodeRabbit
WalkthroughThe wallet preserves cached unfiltered credit when calculating exclude-locked credit. ChangesWallet balance calculations
Estimated code review effort: 3 (Moderate) | ~15–30 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
coderabbitai
Bot
added
size:M
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d449bd84a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (pcoin->IsTrusted()) { | ||
| const CAmount nAvailable = pcoin->GetAvailableCredit(); | ||
| balance += nAvailable; |
There was a problem hiding this comment.
This behavior change replaces four wallet-balance scans and changes available-credit cache handling, but the commit adds no test coverage. Add a targeted wallet unit test that compares TryGetBalances with the individual getters for trusted, unconfirmed, immature, locked, and unlocked outputs, and verifies that an exclude-locked scan preserves the unfiltered cached value; otherwise regressions in this high-risk wallet accounting path will only surface at runtime.
AGENTS.md reference: AGENTS.md:L240-L245
Useful? React with 👍 / 👎.
Cover CWallet::TryGetBalances, which computes the four GUI balances in a single scan of mapWallet, by checking it against the individual getters (GetBalance, GetUnconfirmedBalance, GetImmatureBalance, GetBalance(true)) for trusted, immature-coinbase, unconfirmed (in and out of the mempool), zero-conf self-send, locked and unlocked outputs, with exact expected amounts at every stage. Also pin the CWalletTx::GetAvailableCredit cache contract: the exclude-locked variant must neither overwrite nor invalidate the cached unfiltered credit, and must never be served from the cache. Requested in review of #1896. Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01FKeafjZzU2tPsbLyjZJQ8T
2 participants
PR intention
Remove the GUI stutter that pulses in step with block arrivals on large wallets. The 250 ms balance poll (
WalletModel::checkBalanceChanged) recomputes balances on the GUI thread whenever the block height changes, andCWallet::TryGetBalancesperformed four full scans ofmapWallet—GetBalance(),GetUnconfirmedBalance(),GetImmatureBalance(), andGetBalance(true)for the mintable amount — callingIsTrusted()up to three times per transaction.The mintable scan was the worst offender: the exclude-locked variant of
CWalletTx::GetAvailableCreditcan never be served from the per-transaction credit cache, and it also overwrote and then invalidated that cache on every call, so the next poll's available scan recomputed every transaction's credit from scratch as well.Code changes brief
TryGetBalancesnow computes all four balances in a single scan ofmapWallet, callingIsTrusted()once per transaction. The per-transaction conditions and credit calls are copied verbatim from the replaced getters, so the computed amounts are identical.setLockedCoinsempty), the mintable sum reuses the available credit instead of a second, uncacheableGetAvailableCredit(true, true)computation — excluding locked coins only skips outputs insetLockedCoins, so the values are equal by definition.GetAvailableCreditno longer touches the credit cache whenfExcludeLockedis set; that variant never reads the cache, and destroying it forced constant recomputation. The individual getters (GetBalanceetc.) are unchanged for all other callers.Testing
git diff --checkGetBalance,GetUnconfirmedBalance,GetImmatureBalanceandGetBalance(true)implementations; locks (cs_main/cs_walletvia try-lock) are held across the scan exactly as before.🤖 Generated with Claude Code
https://claude.ai/code/session_01TV7dpUCcTQrSSxnXaR7Xuq
Generated by Claude Code