test: Improve getbalance minconf behavior documentation and testing #32974

1. 
   fgarau commented at 10:20 pm on July 14, 2025: none

   Enhances the wallet_balance.py test to document better and test the current incorrect behaviour of getbalance with minconf parameter.

   The getbalance RPC with minconf incorrectly excludes coins that have been spent, even when those coins had sufficient confirmations when they existed. This commit:

   • Adds detailed comments explaining current vs expected behaviour
   • Creates a comprehensive test method that demonstrates the issue
   • Documents what the correct behaviour should be when the bug is fixed
   • Provides clear test cases for future developers working on the fix

   The test shows that getbalance(minconf=N) should consider all coins that had N or more confirmations when they existed, regardless of whether they were later spent.

2. DrahtBot added the label Tests on Jul 14, 2025
3. 
   DrahtBot commented at 10:20 pm on July 14, 2025: contributor

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

   Code Coverage & Benchmarks

   For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32974.

   Reviews

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

4. test: Improve getbalance minconf behavior documentation and testing

   Enhances the wallet_balance.py test to better document and test the
   current incorrect behavior of getbalance with minconf parameter.
   
   The getbalance RPC with minconf incorrectly excludes coins that have
   been spent, even when those coins had sufficient confirmations when
   they existed. This commit:
   
   - Adds detailed comments explaining current vs expected behavior
   - Creates a comprehensive test method that demonstrates the issue
   - Documents what the correct behavior should be when the bug is fixed
   - Provides clear test cases for future developers working on the fix
   
   The test shows that getbalance(minconf=N) should consider all coins
   that had N or more confirmations when they existed, regardless of
   whether they were later spent.

   fdd6b032ae
5. fgarau force-pushed on Jul 14, 2025
6. DrahtBot added the label CI failed on Jul 14, 2025
7. 
   DrahtBot commented at 10:58 pm on July 14, 2025: contributor

   🚧 At least one of the CI tasks failed. Task lint: https://github.com/bitcoin/bitcoin/runs/45965026864 LLM reason (✨ experimental): The CI failure is caused by lint errors related to trailing whitespace, indicating code style issues rather than build or test failures.

   Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

   • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

   • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

   • An intermittent issue.

   Leave a comment here, if you need help tracking down a confusing failure.

8. DrahtBot removed the label CI failed on Jul 15, 2025
9. 
   luke-jr commented at 3:36 am on July 15, 2025: member

   Spent coins should never be included in the balance (though the change should be)

   However, getbalance has basically been completely broken in Core since 2018. My last attempt to fix it “properly” was here: #14602 (but there were still bugs)

   Knots has a mostly-functional getbalance by reverting the 2018 changes: https://github.com/luke-jr/bitcoin/tree/bugfix_rpc_getbalance_hacky

   Happy to discuss at more length if you’re up for fixing getbalance.

10. 
    l0rinc commented at 3:59 am on July 15, 2025: contributor

    Spent coins should never be included in the balance

    Spentness checks are still problematic all over coins caching as well - not sure it helps, but @andrewtoth attempted a cleanup in https://github.com/bitcoin/bitcoin/pull/30673

11. 
    fgarau commented at 6:13 am on July 15, 2025: none

    Closing to make additional improvements before resubmission
12. fgarau closed this on Jul 15, 2025

    ---

Contributors
fgarau DrahtBot luke-jr l0rinc

Labels
Tests