Skip to content

Commit 187504b

Browse files
committed
Merge bitcoin/bitcoin#23662: rpc: improve getreceivedby{address,label} performance
f336ff7 rpc: avoid expensive `IsMine` calls in `GetReceived` tally (Sebastian Falbesoner) a7b65af 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 f336ff7 w0xlt: ACK bitcoin/bitcoin@f336ff7 furszy: Code ACK f336ff7 Tree-SHA512: 9cbf402b9e269713bd3feda9e31719d9dca8a0dfd526de12fd3d561711589195d0c50143432c65dae279c4eab90a4fc3f99e29fbc0452fefe05113e92d129b8f
2 parents 07cb4de + f336ff7 commit 187504b

File tree

1 file changed

+9
-5
lines changed

1 file changed

+9
-5
lines changed

src/wallet/rpc/coins.cpp

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,17 @@
1818
namespace wallet {
1919
static CAmount GetReceived(const CWallet& wallet, const UniValue& params, bool by_label) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet)
2020
{
21-
std::set<CTxDestination> address_set;
21+
std::set<CScript> output_scripts;
2222

2323
if (by_label) {
2424
// Get the set of addresses assigned to label
2525
std::string label = LabelFromValue(params[0]);
26-
address_set = wallet.GetLabelAddresses(label);
26+
for (const auto& address : wallet.GetLabelAddresses(label)) {
27+
auto output_script{GetScriptForDestination(address)};
28+
if (wallet.IsMine(output_script)) {
29+
output_scripts.insert(output_script);
30+
}
31+
}
2732
} else {
2833
// Get the address
2934
CTxDestination dest = DecodeDestination(params[0].get_str());
@@ -34,7 +39,7 @@ static CAmount GetReceived(const CWallet& wallet, const UniValue& params, bool b
3439
if (!wallet.IsMine(script_pub_key)) {
3540
throw JSONRPCError(RPC_WALLET_ERROR, "Address not found in wallet");
3641
}
37-
address_set.insert(dest);
42+
output_scripts.insert(script_pub_key);
3843
}
3944

4045
// Minimum confirmations
@@ -66,8 +71,7 @@ static CAmount GetReceived(const CWallet& wallet, const UniValue& params, bool b
6671
}
6772

6873
for (const CTxOut& txout : wtx.tx->vout) {
69-
CTxDestination address;
70-
if (ExtractDestination(txout.scriptPubKey, address) && wallet.IsMine(address) && address_set.count(address)) {
74+
if (output_scripts.count(txout.scriptPubKey) > 0) {
7175
amount += txout.nValue;
7276
}
7377
}

0 commit comments

Comments
 (0)