[wallet] [refactor] Simplify getbalance implementation #9614

1. 
   ryanofsky commented at 5:00 pm on January 23, 2017: member

   This is just code cleanup, no behavior is changing. It deduplicates getbalance code and makes it comprehsible so it is not doing bizarre things like subtracting negative fees (https://github.com/bitcoin/bitcoin/pull/9167).

   For each wallet transaction, the balance is updated as follows:

    0+        CAmount debit = wtx.GetDebit(filter);
    1+        const bool outgoing = debit > 0;
    2+        for (const CTxOut& out : wtx.tx->vout) {
    3+            if (outgoing && IsChange(out)) {
    4+                debit -= out.nValue;
    5+            } else if (IsMine(out) & filter && depth >= minDepth && (!account || *account == GetAccountName(out.scriptPubKey))) {
    6+                balance += out.nValue;
    7+            }
    8+        }
    9+
   10+        if (outgoing && (!account || *account == wtx.strFromAccount)) {
   11+            balance -= debit;
   12+        }
   

2. in src/wallet/rpcwallet.cpp: in 4b49ed65ed outdated

   686@@ -687,6 +687,8 @@ UniValue getbalance(const JSONRPCRequest& request)
   687     if (request.params.size() == 0)
   688         return  ValueFromAmount(pwalletMain->GetBalance());
   689 
   690+    const string* account = request.params[0].get_str() != "*" ? &request.params[0].get_str() : nullptr;
   

   ---

   ---

   

   jonasschnelli commented at 8:12 am on January 24, 2017:

   I think this results in no longer throwing RPC_WALLET_INVALID_ACCOUNT_NAME when someone tries to get balance of "*", instead, it returns back the total balance. But I think this is what we want….

   ---

   

   ryanofsky commented at 4:46 pm on January 24, 2017:

   Yes, this is just keeping the current behavior (line 698 before this commit, which is removed in the next commit).
3. jonasschnelli approved
4. 
   jonasschnelli commented at 8:14 am on January 24, 2017: contributor

   Nice. utACK 4b49ed65ede769617eab048e4bbab6d5219600c6

   I could imagine a follow-up refactoring that get rid of all the individual caches…

    0    mutable bool fDebitCached;
    1    mutable bool fCreditCached;
    2    mutable bool fImmatureCreditCached;
    3    mutable bool fAvailableCreditCached;
    4    mutable bool fWatchDebitCached;
    5    mutable bool fWatchCreditCached;
    6    mutable bool fImmatureWatchCreditCached;
    7    mutable bool fAvailableWatchCreditCached;
    8    mutable bool fChangeCached;
    9    mutable CAmount nDebitCached;
   10    mutable CAmount nCreditCached;
   11    mutable CAmount nImmatureCreditCached;
   12    mutable CAmount nAvailableCreditCached;
   13    mutable CAmount nWatchDebitCached;
   14    mutable CAmount nWatchCreditCached;
   15    mutable CAmount nImmatureWatchCreditCached;
   16    mutable CAmount nAvailableWatchCreditCached;
   17    mutable CAmount nChangeCached;
   

   … and manages them in a std::map or similar.

5. jonasschnelli added the label Refactoring on Jan 24, 2017
6. jonasschnelli added the label Wallet on Jan 24, 2017
7. 
   ryanofsky commented at 4:51 pm on January 24, 2017: member

   I could imagine a follow-up refactoring that get rid of all the individual caches…

   I also think this could be good to unify/organize these. #9167 adds two more caches, so they could be integrated as well.

8. ryanofsky force-pushed on Feb 6, 2017
9. 
   ryanofsky commented at 5:45 pm on February 6, 2017: member

   Rebased 4b49ed65ede769617eab048e4bbab6d5219600c6 -> e8ce6a4877186940e95e67aeaa12fd11a23b1e0d (getbalance-cleanup.0 -> getbalance-cleanup.1), fixing trivial merge conflict with #9613.
10. ryanofsky force-pushed on Mar 1, 2017
11. 
    ryanofsky commented at 10:38 am on March 1, 2017: member

    Rebased e8ce6a4877186940e95e67aeaa12fd11a23b1e0d -> be74f34671ac89feaafe80b3ac43ff108c9ceb29 (getbalance-cleanup.1 -> getbalance-cleanup.2) because of conflict with #9764.
12. ryanofsky force-pushed on Mar 3, 2017
13. 
    ryanofsky commented at 7:40 pm on March 3, 2017: member

    Rebased be74f34671ac89feaafe80b3ac43ff108c9ceb29 -> 0b9520d1fb242864b8bd3f6ec68fa91b231467df (pr/getbalance-cleanup.2 -> pr/getbalance-cleanup.3) because of conflicts with pwalletMain renames in #9775.
14. 
    laanwj commented at 9:59 am on April 26, 2017: member

    Needs rebase again (sorry!)

    utACK otherwise.

15. laanwj approved
16. [wallet] Add GetLegacyBalance method to simplify getbalance RPC

    This adds a simpler new implementation of getbalance logic along with asserts
    to confirm it behaves identically to the old logic. The old logic is removed in
    the next commit.

    82b7dc373a
17. [wallet] Remove unneeded legacy getbalance code 02d9f50d5f
18. ryanofsky force-pushed on Apr 26, 2017
19. 
    ryanofsky commented at 10:43 am on April 26, 2017: member

    No problem, thanks for the review!

    Rebased 0b9520d1fb242864b8bd3f6ec68fa91b231467df -> 02d9f50d5f3c96fe888c230d59c5afdab4c7c6a3 (pr/getbalance-cleanup.3 -> pr/getbalance-cleanup.4). No changes except adding another std:: prefix and passing the right new argument to the CWalletDB constructor.

20. laanwj merged this on Apr 26, 2017
21. laanwj closed this on Apr 26, 2017

    ---

22. laanwj referenced this in commit cf5782508a on Apr 26, 2017
23. PastaPastaPasta referenced this in commit 98aaf89f31 on Jun 10, 2019
24. PastaPastaPasta referenced this in commit fa371d48d0 on Jun 10, 2019
25. PastaPastaPasta referenced this in commit 707771d2c5 on Jun 10, 2019
26. PastaPastaPasta referenced this in commit a000739264 on Jun 10, 2019
27. PastaPastaPasta referenced this in commit f9377c3733 on Jun 11, 2019
28. PastaPastaPasta referenced this in commit c61d7aab90 on Jun 11, 2019
29. PastaPastaPasta referenced this in commit 1e8f4aa668 on Jun 15, 2019
30. PastaPastaPasta referenced this in commit 7307824565 on Jun 19, 2019
31. PastaPastaPasta referenced this in commit d4ef11dd2b on Jun 19, 2019
32. PastaPastaPasta referenced this in commit 4f74441e1b on Jun 19, 2019
33. PastaPastaPasta referenced this in commit e92366fb35 on Jun 19, 2019
34. PastaPastaPasta referenced this in commit 95f3b8dcb6 on Jun 19, 2019
35. PastaPastaPasta referenced this in commit e5673dd880 on Jun 20, 2019
36. PastaPastaPasta referenced this in commit 342da6828b on Jun 22, 2019
37. PastaPastaPasta referenced this in commit 29de7f8f31 on Jun 22, 2019
38. PastaPastaPasta referenced this in commit 97ce6f7722 on Jun 22, 2019
39. PastaPastaPasta referenced this in commit 766e3afcfa on Jun 22, 2019
40. PastaPastaPasta referenced this in commit 955a5049f7 on Jun 22, 2019
41. PastaPastaPasta referenced this in commit de9629553c on Jun 24, 2019
42. barrystyle referenced this in commit 9afcbe8503 on Jan 22, 2020
43. Fuzzbawls referenced this in commit c6db2bb845 on Jul 5, 2020
44. Fuzzbawls referenced this in commit 7b2789e948 on Jul 8, 2020
45. Fuzzbawls referenced this in commit 57148f2037 on Jul 8, 2020
46. zkbot referenced this in commit 5b194067ea on Aug 12, 2021
47. MarcoFalke locked this on Sep 8, 2021

Contributors
ryanofsky jonasschnelli laanwj

Labels
Refactoring Wallet