Avoid unnecessary signing provider copies on descriptor expansion

src/Makefile.bench.include

@@ -21,6 +21,7 @@ bench_bench_bitcoin_SOURCES = \
   bench/checkqueue.cpp \
   bench/data.h \
   bench/data.cpp \
+  bench/descriptors.cpp \
   bench/duplicate_inputs.cpp \
   bench/examples.cpp \
   bench/rollingbloom.cpp \

src/bench/descriptors.cpp

@@ -0,0 +1,30 @@
+// Copyright (c) 2019 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <bench/bench.h>
+#include <key.h>
+#include <script/descriptor.h>
+#include <script/standard.h>
+
+#include <string>
+#include <utility>
+
+static void ExpandDescriptor(benchmark::Bench& bench)
+{
+    const auto desc_str = "sh(wsh(multi(16,03669b8afcec803a0d323e9a17f3ea8e68e8abe5a278020a929adbec52421adbd0,0260b2003c386519fc9eadf2b5cf124dd8eea4c4e68d5e154050a9346ea98ce600,0362a74e399c39ed5593852a30147f2959b56bb827dfa3e60e464b02ccf87dc5e8,0261345b53de74a4d721ef877c255429961b7e43714171ac06168d7e08c542a8b8,02da72e8b46901a65d4374fe6315538d8f368557dda3a1dcf9ea903f3afe7314c8,0318c82dd0b53fd3a932d16e0ba9e278fcc937c582d5781be626ff16e201f72286,0297ccef1ef99f9d73dec9ad37476ddb232f1238aff877af19e72ba04493361009,02e502cfd5c3f972fe9a3e2a18827820638f96b6f347e54d63deb839011fd5765d,03e687710f0e3ebe81c1037074da939d409c0025f17eb86adb9427d28f0f7ae0e9,02c04d3a5274952acdbc76987f3184b346a483d43be40874624b29e3692c1df5af,02ed06e0f418b5b43a7ec01d1d7d27290fa15f75771cb69b642a51471c29c84acd,036d46073cbb9ffee90473f3da429abc8de7f8751199da44485682a989a4bebb24,02f5d1ff7c9029a80a4e36b9a5497027ef7f3e73384a4a94fbfe7c4e9164eec8bc,02e41deffd1b7cce11cde209a781adcffdabd1b91c0ba0375857a2bfd9302419f3,02d76625f7956a7fc505ab02556c23ee72d832f1bac391bcd2d3abce5710a13d06,0399eb0a5487515802dc14544cf10b3666623762fbed2ec38a3975716e2c29c232)))";
+    const std::pair<int64_t, int64_t> range = {0, 1000};
+    FlatSigningProvider provider;
+    std::string error;
+    auto desc = Parse(desc_str, provider, error);
+
+    bench.run([&] {
+        for (int i = range.first; i <= range.second; ++i) {
+            std::vector<CScript> scripts;
+            bool success = desc->Expand(i, provider, scripts, provider);
+            assert(success);
+        }
+    });
+}
+
+BENCHMARK(ExpandDescriptor);

src/script/descriptor.cpp

@@ -622,7 +622,7 @@ class DescriptorImpl : public Descriptor
             assert(outscripts.size() == 1);
             subscripts.emplace_back(std::move(outscripts[0]));
         }
-        out = Merge(std::move(out), std::move(subprovider));
+        out.Merge(subprovider);
 
         std::vector<CPubKey> pubkeys;
         pubkeys.reserve(entries.size());

src/script/signingprovider.cpp

@@ -64,22 +64,15 @@ bool FlatSigningProvider::GetTaprootSpendData(const XOnlyPubKey& output_key, Tap
     return LookupHelper(tr_spenddata, output_key, spenddata);
 }
 
-FlatSigningProvider Merge(const FlatSigningProvider& a, const FlatSigningProvider& b)
-{
-    FlatSigningProvider ret;
-    ret.scripts = a.scripts;
-    ret.scripts.insert(b.scripts.begin(), b.scripts.end());
-    ret.pubkeys = a.pubkeys;
-    ret.pubkeys.insert(b.pubkeys.begin(), b.pubkeys.end());
-    ret.keys = a.keys;
-    ret.keys.insert(b.keys.begin(), b.keys.end());
-    ret.origins = a.origins;
-    ret.origins.insert(b.origins.begin(), b.origins.end());
-    ret.tr_spenddata = a.tr_spenddata;
-    for (const auto& [output_key, spenddata] : b.tr_spenddata) {
-        ret.tr_spenddata[output_key].Merge(spenddata);
+void FlatSigningProvider::Merge(const FlatSigningProvider& other)
+{
+    scripts.insert(other.scripts.begin(), other.scripts.end());
+    pubkeys.insert(other.pubkeys.begin(), other.pubkeys.end());
+    keys.insert(other.keys.begin(), other.keys.end());
+    origins.insert(other.origins.begin(), other.origins.end());
+    for (const auto& [output_key, spenddata] : other.tr_spenddata) {
+        tr_spenddata[output_key].Merge(spenddata);
     }
-    return ret;
 }
 
 void FillableSigningProvider::ImplicitlyLearnRelatedKeyScripts(const CPubKey& pubkey)

src/script/signingprovider.h

@@ -83,9 +83,9 @@ struct FlatSigningProvider final : public SigningProvider
     bool GetKeyOrigin(const CKeyID& keyid, KeyOriginInfo& info) const override;
     bool GetKey(const CKeyID& keyid, CKey& key) const override;
     bool GetTaprootSpendData(const XOnlyPubKey& output_key, TaprootSpendData& spenddata) const override;
-};
 
-FlatSigningProvider Merge(const FlatSigningProvider& a, const FlatSigningProvider& b);
+    void Merge(const FlatSigningProvider& other);
+};
 
 /** Fillable signing provider that keeps keys in an address->secret map */
 class FillableSigningProvider : public SigningProvider

src/test/descriptor_tests.cpp

@@ -234,13 +234,17 @@ void DoCheck(const std::string& prv, const std::string& pub, const std::string&
             // For each of the produced scripts, verify solvability, and when possible, try to sign a transaction spending it.
             for (size_t n = 0; n < spks.size(); ++n) {
                 BOOST_CHECK_EQUAL(ref[n], HexStr(spks[n]));
-                BOOST_CHECK_EQUAL(IsSolvable(Merge(key_provider, script_provider), spks[n]), (flags & UNSOLVABLE) == 0);
+                FlatSigningProvider solve_provider{key_provider};
+                solve_provider.Merge(script_provider);
+                BOOST_CHECK_EQUAL(IsSolvable(solve_provider, spks[n]), (flags & UNSOLVABLE) == 0);
 
                 if (flags & SIGNABLE) {
                     CMutableTransaction spend;
                     spend.vin.resize(1);
                     spend.vout.resize(1);
-                    BOOST_CHECK_MESSAGE(SignSignature(Merge(keys_priv, script_provider), spks[n], spend, 0, 1, SIGHASH_ALL), prv);
+                    FlatSigningProvider sign_provider{keys_priv};
+                    sign_provider.Merge(script_provider);
+                    BOOST_CHECK_MESSAGE(SignSignature(sign_provider, spks[n], spend, 0, 1, SIGHASH_ALL), prv);
                 }
 
                 /* Infer a descriptor from the generated script, and verify its solvability and that it roundtrips. */

src/wallet/scriptpubkeyman.cpp

@@ -2054,7 +2054,7 @@ bool DescriptorScriptPubKeyMan::SignTransaction(CMutableTransaction& tx, const s
         if (!coin_keys) {
             continue;
         }
-        *keys = Merge(*keys, *coin_keys);
+        keys->Merge(*coin_keys);
     }
 
     return ::SignTransaction(tx, keys.get(), coins, sighash, input_errors);
@@ -2115,15 +2115,15 @@ TransactionError DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTransaction&
         std::unique_ptr<FlatSigningProvider> keys = std::make_unique<FlatSigningProvider>();
         std::unique_ptr<FlatSigningProvider> script_keys = GetSigningProvider(script, sign);
         if (script_keys) {
-            *keys = Merge(*keys, *script_keys);
+            keys->Merge(*script_keys);
         } else {
             // Maybe there are pubkeys listed that we can sign for
             script_keys = std::make_unique<FlatSigningProvider>();
             for (const auto& pk_pair : input.hd_keypaths) {
                 const CPubKey& pubkey = pk_pair.first;
                 std::unique_ptr<FlatSigningProvider> pk_keys = GetSigningProvider(pubkey);
                 if (pk_keys) {
-                    *keys = Merge(*keys, *pk_keys);
+                    keys->Merge(*pk_keys);
                 }
             }
         }