Simplify calls to retrieve credit and balance #8158

pull joaopaulofonseca wants to merge 1 commits into bitcoin:master from uphold:enhancement/unification-wallet-balance changing 3 files +50 −124

joaopaulofonseca commented at 9:18 AM on June 7, 2016: contributor

In order to retrieve the different balance values, CWallet uses several similar methods, which differ only if the balance takes watch-only addresses in account or not.

Examples: CWallet::GetBalance() CWallet::GetWatchOnlyBalance() CWallet::GetUnconfirmedBalance() CWallet::GetUnconfirmedWatchOnlyBalance() (...)

This PR simplifies these calls, removing some of the duplicate methods and passing the isminetype to compute the correct balance value.
[Wallet] Simplified calls to retrieve credit and balance f717be3c1e
jonasschnelli added the label Wallet on Jun 7, 2016
jonasschnelli commented at 9:21 AM on June 7, 2016: contributor

Concept ACK. The current code is messy.
laanwj commented at 11:57 AM on June 7, 2016: member

Concept ACK, looks like a good idea.
joaopaulofonseca commented at 3:53 PM on June 8, 2016: contributor

@jonasschnelli yes, a little 😉. Although this is not a performance optimization, I think this piece of code is cleaner this way.
laanwj commented at 1:08 PM on June 22, 2016: member

It certainly does. wallet.cpp has a lot of this kind of redundant code, or different code paths of achieving (approximately) the same, such as the different ways of getting the balance that return different values in different circumstances (getbalance versus getbalance * versus...). Good to start cleaning this up.

Does need to be reviewed carefully of course.
joaopaulofonseca commented at 1:43 PM on June 22, 2016: contributor

@laanwj indeed. wallet.cpp is kinda messy and hard to dig in, due to all this redundant code. I'll try to improve some other code paths as well.

in src/wallet/wallet.cpp:None in f717be3c1e

1381 | -{
1382 | -    if (pwallet == 0)
1383 | -        return 0;
1384 | +    if (IsCoinBase() && GetBlocksToMaturity() > 0 && IsInMainChain()) {
1385 | +        CAmount credit = 0;
1386 | +        if (filter & ISMINE_SPENDABLE) {

sipa commented at 12:12 PM on August 25, 2016:

What if filter is ISMINE_ALL? I'd prefer to see either an exact equivalent check, or a switch case, or an assert to prevent unhandled cases.

laanwj commented at 11:30 AM on November 7, 2016: member

Looks like this PR is inactive. Let me know if you start work on this again then I'll re-open.
laanwj closed this on Nov 7, 2016
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels