From fa3f15f2dd94ae597a66037f5928fe4e90fe099d Mon Sep 17 00:00:00 2001
From: MacroFake <falke.marco@gmail.com>
Date: Sat, 30 Jul 2022 14:27:47 +0200
Subject: [PATCH] refactor: Avoid copies in FlatSigningProvider Merge

---
 src/script/descriptor.cpp      |  2 +-
 src/script/signingprovider.cpp | 20 +++++++-------------
 src/script/signingprovider.h   |  5 +++--
 src/test/descriptor_tests.cpp  |  2 +-
 src/wallet/rpc/spend.cpp       |  2 +-
 src/wallet/scriptpubkeyman.cpp | 10 +++++-----
 6 files changed, 18 insertions(+), 23 deletions(-)

diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp
index 9bcbe1ceefb..8ace9c3136a 100644
--- a/src/script/descriptor.cpp
+++ b/src/script/descriptor.cpp
@@ -644,7 +644,7 @@ public:
             assert(outscripts.size() == 1);
             subscripts.emplace_back(std::move(outscripts[0]));
         }
-        out = Merge(std::move(out), std::move(subprovider));
+        out.Merge(std::move(subprovider));
 
         std::vector<CPubKey> pubkeys;
         pubkeys.reserve(entries.size());
diff --git a/src/script/signingprovider.cpp b/src/script/signingprovider.cpp
index c624a176286..a82a8b252a1 100644
--- a/src/script/signingprovider.cpp
+++ b/src/script/signingprovider.cpp
@@ -77,20 +77,14 @@ bool FlatSigningProvider::GetTaprootBuilder(const XOnlyPubKey& output_key, Tapro
     return LookupHelper(tr_trees, output_key, builder);
 }
 
-FlatSigningProvider Merge(const FlatSigningProvider& a, const FlatSigningProvider& b)
+FlatSigningProvider& FlatSigningProvider::Merge(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_trees = a.tr_trees;
-    ret.tr_trees.insert(b.tr_trees.begin(), b.tr_trees.end());
-    return ret;
+    scripts.merge(b.scripts);
+    pubkeys.merge(b.pubkeys);
+    keys.merge(b.keys);
+    origins.merge(b.origins);
+    tr_trees.merge(b.tr_trees);
+    return *this;
 }
 
 void FillableSigningProvider::ImplicitlyLearnRelatedKeyScripts(const CPubKey& pubkey)
diff --git a/src/script/signingprovider.h b/src/script/signingprovider.h
index 792cc903f2b..2d4234ea0b4 100644
--- a/src/script/signingprovider.h
+++ b/src/script/signingprovider.h
@@ -6,6 +6,7 @@
 #ifndef BITCOIN_SCRIPT_SIGNINGPROVIDER_H
 #define BITCOIN_SCRIPT_SIGNINGPROVIDER_H
 
+#include <attributes.h>
 #include <key.h>
 #include <pubkey.h>
 #include <script/keyorigin.h>
@@ -85,9 +86,9 @@ struct FlatSigningProvider final : public SigningProvider
     bool GetKey(const CKeyID& keyid, CKey& key) const override;
     bool GetTaprootSpendData(const XOnlyPubKey& output_key, TaprootSpendData& spenddata) const override;
     bool GetTaprootBuilder(const XOnlyPubKey& output_key, TaprootBuilder& builder) const override;
-};
 
-FlatSigningProvider Merge(const FlatSigningProvider& a, const FlatSigningProvider& b);
+    FlatSigningProvider& Merge(FlatSigningProvider&& b) LIFETIMEBOUND;
+};
 
 /** Fillable signing provider that keeps keys in an address->secret map */
 class FillableSigningProvider : public SigningProvider
diff --git a/src/test/descriptor_tests.cpp b/src/test/descriptor_tests.cpp
index 1eb4b373b47..d9503d7624a 100644
--- a/src/test/descriptor_tests.cpp
+++ b/src/test/descriptor_tests.cpp
@@ -312,7 +312,7 @@ void DoCheck(const std::string& prv, const std::string& pub, const std::string&
                     txdata.Init(spend, std::move(utxos), /*force=*/true);
                     MutableTransactionSignatureCreator creator{spend, 0, CAmount{0}, &txdata, SIGHASH_DEFAULT};
                     SignatureData sigdata;
-                    BOOST_CHECK_MESSAGE(ProduceSignature(Merge(keys_priv, script_provider), creator, spks[n], sigdata), prv);
+                    BOOST_CHECK_MESSAGE(ProduceSignature(FlatSigningProvider{keys_priv}.Merge(FlatSigningProvider{script_provider}), creator, spks[n], sigdata), prv);
                 }
 
                 /* Infer a descriptor from the generated script, and verify its solvability and that it roundtrips. */
diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp
index 628bf3cc998..72567e3f078 100644
--- a/src/wallet/rpc/spend.cpp
+++ b/src/wallet/rpc/spend.cpp
@@ -644,7 +644,7 @@ void FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& fee_out,
                     throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Unable to parse descriptor '%s': %s", desc_str, error));
                 }
                 desc->Expand(0, desc_out, scripts_temp, desc_out);
-                coinControl.m_external_provider = Merge(coinControl.m_external_provider, desc_out);
+                coinControl.m_external_provider.Merge(std::move(desc_out));
             }
         }
     }
diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp
index 7ff017775e0..ce8376fc7f5 100644
--- a/src/wallet/scriptpubkeyman.cpp
+++ b/src/wallet/scriptpubkeyman.cpp
@@ -2080,7 +2080,7 @@ std::unique_ptr<FlatSigningProvider> DescriptorScriptPubKeyMan::GetSigningProvid
     // Fetch SigningProvider from cache to avoid re-deriving
     auto it = m_map_signing_providers.find(index);
     if (it != m_map_signing_providers.end()) {
-        *out_keys = Merge(*out_keys, it->second);
+        out_keys->Merge(FlatSigningProvider{it->second});
     } else {
         // Get the scripts, keys, and key origins for this script
         std::vector<CScript> scripts_temp;
@@ -2117,7 +2117,7 @@ bool DescriptorScriptPubKeyMan::SignTransaction(CMutableTransaction& tx, const s
         if (!coin_keys) {
             continue;
         }
-        *keys = Merge(*keys, *coin_keys);
+        keys->Merge(std::move(*coin_keys));
     }
 
     return ::SignTransaction(tx, keys.get(), coins, sighash, input_errors);
@@ -2178,7 +2178,7 @@ 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(std::move(*script_keys));
         } else {
             // Maybe there are pubkeys listed that we can sign for
             script_keys = std::make_unique<FlatSigningProvider>();
@@ -2186,7 +2186,7 @@ TransactionError DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTransaction&
                 const CPubKey& pubkey = pk_pair.first;
                 std::unique_ptr<FlatSigningProvider> pk_keys = GetSigningProvider(pubkey);
                 if (pk_keys) {
-                    *keys = Merge(*keys, *pk_keys);
+                    keys->Merge(std::move(*pk_keys));
                 }
             }
             for (const auto& pk_pair : input.m_tap_bip32_paths) {
@@ -2198,7 +2198,7 @@ TransactionError DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTransaction&
                     fullpubkey.Set(b, b + 33);
                     std::unique_ptr<FlatSigningProvider> pk_keys = GetSigningProvider(fullpubkey);
                     if (pk_keys) {
-                        *keys = Merge(*keys, *pk_keys);
+                        keys->Merge(std::move(*pk_keys));
                     }
                 }
             }