diff options
author | Andrew Chow <achow101-github@achow101.com> | 2022-05-16 14:28:31 -0400 |
---|---|---|
committer | Andrew Chow <achow101-github@achow101.com> | 2022-05-16 14:35:42 -0400 |
commit | 187504b038cc1151e5813ac56b79c603439d352c (patch) | |
tree | 2484da76a3f77704c9f7aea270f5b91b638785f5 | |
parent | 07cb4dee5d12657e9cf33c442678e9a1cd2cec4a (diff) | |
parent | f336ff7f213564909cf5f9742618cc6ec87600fd (diff) |
Merge bitcoin/bitcoin#23662: rpc: improve `getreceivedby{address,label}` performance
f336ff7f213564909cf5f9742618cc6ec87600fd rpc: avoid expensive `IsMine` calls in `GetReceived` tally (Sebastian Falbesoner)
a7b65af2a4450663d4a20a617c59dac87b34fb36 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 f336ff7f213564909cf5f9742618cc6ec87600fd
w0xlt:
ACK https://github.com/bitcoin/bitcoin/pull/23662/commits/f336ff7f213564909cf5f9742618cc6ec87600fd
furszy:
Code ACK f336ff7f
Tree-SHA512: 9cbf402b9e269713bd3feda9e31719d9dca8a0dfd526de12fd3d561711589195d0c50143432c65dae279c4eab90a4fc3f99e29fbc0452fefe05113e92d129b8f
-rw-r--r-- | src/wallet/rpc/coins.cpp | 14 |
1 files changed, 9 insertions, 5 deletions
diff --git a/src/wallet/rpc/coins.cpp b/src/wallet/rpc/coins.cpp index 4eccff3969..bd61c9c62f 100644 --- a/src/wallet/rpc/coins.cpp +++ b/src/wallet/rpc/coins.cpp @@ -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; } } |