getreceivedbyaddress and getreceivedbylabel into a single function GetReceived. This reduces repeated code and makes it similar to listreceivedbyaddress and listreceivedbylabel, which use the function ListReceived. It will also make the change in #14707 simpler and easier to review.
582@@ -583,6 +583,57 @@ static UniValue signmessage(const JSONRPCRequest& request)
583 return EncodeBase64(vchSig.data(), vchSig.size());
584 }
585
586+static UniValue GetReceived(interfaces::Chain::Lock& locked_chain, CWallet * const wallet, const UniValue& params, bool by_label) EXCLUSIVE_LOCKS_REQUIRED(wallet->cs_wallet)
CAmount, so that (if needed) the function can also be called from unit tests easily.
CAmount makes it easier to test? Is it due to not having UniValue as a dependency? In that case I should remove UniValue as a parameter as well.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Reviewers, this pull request conflicts with the following ones:
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.
623+ }
624+
625+ for (const CTxOut& txout : wtx.tx->vout) {
626+ CTxDestination address;
627+ if (ExtractDestination(txout.scriptPubKey, address) && wallet->IsMine(address) && addresses.count(address)) {
628+ amount += txout.nValue;
getreceivedbylabel, and the singular case used by getreceivedbyaddress is no longer needed.
582@@ -583,6 +583,57 @@ static UniValue signmessage(const JSONRPCRequest& request)
583 return EncodeBase64(vchSig.data(), vchSig.size());
584 }
585
586+static CAmount GetReceived(interfaces::Chain::Lock& locked_chain, CWallet * const wallet, const UniValue& params, bool by_label) EXCLUSIVE_LOCKS_REQUIRED(wallet->cs_wallet)
wallet/pwallet (see e.g. ListReceived() and SendMoney() in the same file) to be more consistent
628+ amount += txout.nValue;
629+ }
630+ }
631+ }
632+
633+ return amount;
return
618+ }
619+
620+ int depth = wtx.GetDepthInMainChain();
621+ if (depth < min_depth) {
622+ continue;
623+ }
More a matter of taste, but this could simply be put in the if condition above and without an extra variable (that is used only once anyway)? Like
0if (wtx.IsCoinBase() || !locked_chain.checkFinalTx(*wtx.tx) || wtx.GetDepthInMainChain() < min_depth) {
1 continue;
2}
699- if (wtx.GetDepthInMainChain() >= nMinDepth)
700- nAmount += txout.nValue;
701- }
702-
703- return ValueFromAmount(nAmount);
704+ return ValueFromAmount(GetReceived(*locked_chain, pwallet, request.params, false));
/* by_label */ before boolean parameter false to increase readability.
738- }
739- }
740- }
741-
742- return ValueFromAmount(nAmount);
743+ return ValueFromAmount(GetReceived(*locked_chain, pwallet, request.params, true));
/* by_label */ before boolean parameter true to increase readability.
utACK 4111967281a37cfe0d34802a80dba0715bfe3ffb
Nice cleanup, and also nice to hoist the DepthInMainChain check out of the inner loop.
Optionally, since this is more than a pure code move, some of the variable names/comments could be improved as well (the “addresses” variable name, as well as the comment referring to pubkeys are outdated).
582@@ -583,6 +583,52 @@ static UniValue signmessage(const JSONRPCRequest& request)
583 return EncodeBase64(vchSig.data(), vchSig.size());
584 }
585
586+static CAmount GetReceived(interfaces::Chain::Lock& locked_chain, CWallet * const pwallet, const UniValue& params, bool by_label) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet)
0static CAmount GetReceived(interfaces::Chain::Lock& locked_chain, const CWallet& wallet, const UniValue& params, bool by_label) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet)
598+ }
599+
600+ // Minimum confirmations
601+ int min_depth = 1;
602+ if (!params[1].isNull())
603+ min_depth = params[1].get_int();
0 const int min_depth{params[1].isNull() ? 1 : params[1].get_int()};
can be written shorter