foster/bitcoin-bitcoin-core
0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-03 09:56:38 -05:00
Code Issues Activity

Merge bitcoin/bitcoin#23662: rpc: improve getreceivedby{address,label} performance

Browse source 
f336ff7f21 rpc: avoid expensive `IsMine` calls in `GetReceived` tally (Sebastian Falbesoner)
a7b65af2a4 rpc: avoid scriptPubKey<->CTxDestination conversions in `GetReceived` tally (Sebastian Falbesoner)

Pull request description:

  The RPC calls `getreceivedbyaddress`/`getreceivedbylabel` both use the internal helper function `GetReceived` which was introduced in PR #17579 to deduplicate tallying code. For every wallet-related transaction output, the following unnecessary operations are currently performed in the tally loop, leading to a quite bad performance (as reported in #23645):
  - converting from CScript -> TxDestination (`ExtractDestination(...)`), converting from TxDestination -> CScript (`CWallet::IsMine(const CTxDestination& dest)`); this can be avoided by directly using output scripts in the search set instead of addresses (first commit)
  - checking if the iterated output script belongs to the wallet by calling `IsMine`; this can be avoided by only adding addresses to the search set which fulfil `IsMine` in the first place (second commit)

  ### Benchmark results
  The functional test [wallet_pr23662.py](https://github.com/theStack/bitcoin/blob/pr23662_benchmarks/test/functional/wallet_pr23662.py) (not part of this PR) creates transactions with 15000 different addresses:
  - 5000 outputs (500 txs with 10 outputs each) with label set, IsMine set (received)
  - 5000 outputs (500 txs with 10 outputs each) with label set, IsMine not set (sent)
  - 5000 outputs (500 txs with 10 outputs each) without label set, IsMine not set (sent)

  Then, the time is measured for calling `getreceivedbyaddress` and `getreceivedbylabel`, the latter with two variants. Results on my machine:

  | branch             | `getreceivedbyaddress` (single) | `getreceivedbylabel` (single) | `getreceivedbylabel` (10000) |
  |--------------------|---------------------------------|-------------------------------|------------------------------|
  | master             |             406.13ms            |          425.33ms             |          446.58ms            |
  | PR (first commit)  |             367.18ms            |          365.81ms             |          426.33ms            |
  | PR (second commit) |               3.96ms            |            4.83ms             |          339.69ms            |

  Fixes #23645.

ACKs for top commit:
  achow101:
    ACK f336ff7f21
  w0xlt:
    ACK f336ff7f21
  furszy:
    Code ACK f336ff7f

Tree-SHA512: 9cbf402b9e269713bd3feda9e31719d9dca8a0dfd526de12fd3d561711589195d0c50143432c65dae279c4eab90a4fc3f99e29fbc0452fefe05113e92d129b8f
This commit is contained in:
Andrew Chow 2022-05-16 14:28:31 -04:00
commit 187504b038
No known key found for this signature in database
GPG key ID: 17565732E08E5E41
1 changed files with 9 additions and 5 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

14
src/wallet/rpc/coins.cpp
View file

@ -18,12 +18,17 @@
namespace wallet {
static CAmount GetReceived(const CWallet& wallet, const UniValue& params, bool by_label) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet)
{
std::set<CTxDestination> address_set;
std::set<CScript> output_scripts;
if (by_label) {
// Get the set of addresses assigned to label
std::string label = LabelFromValue(params[0]);
address_set = wallet.GetLabelAddresses(label);
for (const auto& address : wallet.GetLabelAddresses(label)) {
auto output_script{GetScriptForDestination(address)};
if (wallet.IsMine(output_script)) {
output_scripts.insert(output_script);
}
}
} else {
// Get the address
CTxDestination dest = DecodeDestination(params[0].get_str());
@ -34,7 +39,7 @@ static CAmount GetReceived(const CWallet& wallet, const UniValue& params, bool b
if (!wallet.IsMine(script_pub_key)) {
throw JSONRPCError(RPC_WALLET_ERROR, "Address not found in wallet");
}
address_set.insert(dest);
output_scripts.insert(script_pub_key);
}
// Minimum confirmations
@ -66,8 +71,7 @@ static CAmount GetReceived(const CWallet& wallet, const UniValue& params, bool b
}
for (const CTxOut& txout : wtx.tx->vout) {
CTxDestination address;
if (ExtractDestination(txout.scriptPubKey, address) && wallet.IsMine(address) && address_set.count(address)) {
if (output_scripts.count(txout.scriptPubKey) > 0) {
amount += txout.nValue;
}
}