Merge bitcoin/bitcoin#25748: refactor: Avoid copies in FlatSigningProvider Merge

fa3f15f2dd94ae597a66037f5928fe4e90fe099d refactor: Avoid copies in FlatSigningProvider Merge (MacroFake)

Pull request description:

  `Merge` will create several copies unconditionally:
  * To initialize the args `a`, and `b`
  * `ret`, which is the merge of the two args

  So change the code to let the caller decide how many copies they need/want:
  * `a`, and `b` must be explicitly moved or copied by the caller
  * `ret` is no longer needed, as `a` can be used for it in place "for free"

ACKs for top commit:
  achow101:
    ACK fa3f15f2dd94ae597a66037f5928fe4e90fe099d
  furszy:
    looks good, ACK fa3f15f2
  ryanofsky:
    Code review ACK fa3f15f2dd94ae597a66037f5928fe4e90fe099d. Confirmed that all the places `std::move` was added the argument actually did seem safe to move from. Compiler enforces that temporary copies are explicitly created in non-move cases.

Tree-SHA512: 7c027ccdea1549cd9f37403344ecbb76e008adf545f6ce52996bf95e89eb7dc89af6cb31435a9289d6f2eea1c416961b2fb96348bc8a211d550728f1d99ac49c

1 files changed, 5 insertions, 5 deletions

diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp
index 7511312f70..8d40846332 100644
--- a/src/wallet/scriptpubkeyman.cpp
+++ b/src/wallet/scriptpubkeyman.cpp
@@ -2085,7 +2085,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;
@@ -2122,7 +2122,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);
@@ -2183,7 +2183,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>();
@@ -2191,7 +2191,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) {
@@ -2203,7 +2203,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));
                     }
                 }
             }