wallet: Replace `GetBalance()` logic with `AvailableCoins()` #26756

1. 
   w0xlt commented at 4:58 AM on December 27, 2022: contributor

   This PR proposes a solution for #26500 by changing the AvailableCoins() function to calculate values by status (TRUSTED, UNTRUSTED_PENDING and IMMATURE) and ownership (MINE, WATCH_ONLY), eliminating the GetBalance() logic.

   The downside is that the cache is no longer used. Not sure about the performance implication, but if the approach is OK, caching can also be used with this solution.

   This approach also fixes the bug mentioned at the end of the wallet_abandonconflict.py file.

2. 
   DrahtBot commented at 4:58 AM on December 27, 2022: contributor

   <!--e57a25ab6845829454e8d69fc972939a-->

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   <!--021abf342d371248e50ceaed478a90ca-->

   Reviews

   See the guideline for information on the review process. A summary of reviews will appear here.

   <!--174a7506f384e20aa4161008e828411d-->

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #26951 (wallet: improve rescan performance 640% by pstratem)
   • #26573 (Wallet: don't underestimate the fees when spending a Taproot output by darosior)
   • #26567 (Wallet: estimate the size of signed inputs using descriptors by darosior)
   • #26152 (Bump unconfirmed ancestor transactions to target feerate by Xekyo)
   • #25979 ([WIP] wallet: standardize change output detection process by furszy)
   • #25620 (wallet: Introduce AddressBookManager by furszy)
   • #24897 ([Draft / POC] Silent Payments by w0xlt)
   • #24149 (Signing support for Miniscript Descriptors by darosior)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

3. DrahtBot added the label Wallet on Dec 27, 2022
4. w0xlt force-pushed on Dec 27, 2022
5. w0xlt force-pushed on Dec 27, 2022
6. DrahtBot cross-referenced this on Dec 28, 2022 from issue [WIP] wallet: tx creation, don't select outputs from txes that are being replaced by furszy
7. DrahtBot cross-referenced this on Dec 28, 2022 from issue wallet: GetEffectiveBalance by willcl-ark
8. DrahtBot cross-referenced this on Dec 28, 2022 from issue test: clean and extend availablecoins_tests coverage by furszy
9. DrahtBot cross-referenced this on Dec 28, 2022 from issue wallet: simplify ListCoins implementation by furszy
10. DrahtBot cross-referenced this on Dec 28, 2022 from issue wallet: Introduce AddressBookManager by furszy
11. DrahtBot cross-referenced this on Dec 28, 2022 from issue Signing support for Miniscript Descriptors by darosior
12. DrahtBot added the label Needs rebase on Jan 3, 2023
13. wallet: Separate the `AvailableCoins()` function into a new file

    This allows `AvailableCoins()` to be used in multiple files
    without creating a circular dependency

    0cc7da8ff6
14. wallet: Replace `GetBalance()` logic with modified `AvailableCoins()`

    Add to the `AvailableCoins()` function the ability to separate
    values by status and ownership of each coin.

    e57dfaa9d2
15. wallet: Remove unused `CachedTxGetAvailableCredit()` function 3a2bc11c26
16. w0xlt force-pushed on Jan 5, 2023
17. DrahtBot removed the label Needs rebase on Jan 5, 2023
18. 
    w0xlt commented at 12:21 PM on January 5, 2023: contributor

    Rebased.

19. DrahtBot cross-referenced this on Jan 7, 2023 from issue [Draft / POC] Silent Payments by w0xlt
20. DrahtBot cross-referenced this on Jan 10, 2023 from issue [WIP] wallet: standardize change output detection process by furszy
21. DrahtBot cross-referenced this on Jan 23, 2023 from issue wallet: improve rescan performance 640% by pstratem
22. DrahtBot cross-referenced this on Feb 1, 2023 from issue Bump unconfirmed ancestor transactions to target feerate by murchandamus
23. DrahtBot cross-referenced this on Feb 5, 2023 from issue Wallet: don't underestimate the fees when spending a Taproot output by darosior
24. DrahtBot cross-referenced this on Feb 5, 2023 from issue Wallet: estimate the size of signed inputs using descriptors by darosior
25. DrahtBot added the label Needs rebase on Feb 16, 2023
26. 
    DrahtBot commented at 11:07 AM on February 16, 2023: contributor

    <!--cf906140f33d8803c4a75a2196329ecb-->

    🐙 This pull request conflicts with the target branch and needs rebase.

27. achow101 requested review from achow101 on Apr 25, 2023
28. achow101 requested review from josibake on Apr 25, 2023
29. 
    achow101 commented at 12:13 PM on May 1, 2023: member

    I think this will end up making GetBalance really slow. We should really be tracking our UTXOs and computing the balance (and available coins) from that instead of iterating the entire wallet to figure out the UTXOs. #27286 implements the first steps to doing that, and I think we should prefer moving in that direction.

30. 
    josibake commented at 1:10 PM on May 1, 2023: member

    I'd also prefer we stopped using AvailableCoins as a general wallet traversal tool in favor of something like #27286

    A lot of bugs/strangeness in the wallet seems to come from the fact that we always need to iterate over a bunch of transactions to learn the state of our wallet. If we did track wallet state in one place, we could have well-scoped individual functions for specific tasks.

31. 
    achow101 commented at 2:43 PM on May 3, 2023: member

    Closing due to lack of interest in this approach.

32. achow101 closed this on May 3, 2023

    ---

33. bitcoin locked this on May 2, 2024

Contributors

w0xltDrahtBotachow101josibake

Review Requested
achow101josibake