Refactor: Replace SigningProvider pointers with unique_ptrs

Unclear if this change is necessary or useful:
bitcoin#16341 (comment)

This commit does not change behavior.

src/interfaces/wallet.cpp

@@ -122,15 +122,15 @@ class WalletImpl : public Wallet
     }
     bool getPubKey(const CScript& script, const CKeyID& address, CPubKey& pub_key) override
     {
-        const SigningProvider* provider = m_wallet->GetSigningProvider(script);
+        std::unique_ptr<SigningProvider> provider = m_wallet->GetSigningProvider(script);
         if (provider) {
             return provider->GetPubKey(address, pub_key);
         }
         return false;
     }
     bool getPrivKey(const CScript& script, const CKeyID& address, CKey& key) override
     {
-        const SigningProvider* provider = m_wallet->GetSigningProvider(script);
+        std::unique_ptr<SigningProvider> provider = m_wallet->GetSigningProvider(script);
         if (provider) {
             return provider->GetKey(address, key);
         }

src/wallet/psbtwallet.cpp

@@ -51,20 +51,20 @@ TransactionError FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& ps
         }
         SignatureData sigdata;
         input.FillSignatureData(sigdata);
-        const SigningProvider* provider = pwallet->GetSigningProvider(script, sigdata);
+        std::unique_ptr<SigningProvider> provider = pwallet->GetSigningProvider(script, sigdata);
         if (!provider) {
             continue;
         }
 
-        complete &= SignPSBTInput(HidingSigningProvider(provider, !sign, !bip32derivs), psbtx, i, sighash_type);
+        complete &= SignPSBTInput(HidingSigningProvider(provider.get(), !sign, !bip32derivs), psbtx, i, sighash_type);
     }
 
     // Fill in the bip32 keypaths and redeemscripts for the outputs so that hardware wallets can identify change
     for (unsigned int i = 0; i < psbtx.tx->vout.size(); ++i) {
         const CTxOut& out = psbtx.tx->vout.at(i);
-        const SigningProvider* provider = pwallet->GetSigningProvider(out.scriptPubKey);
+        std::unique_ptr<SigningProvider> provider = pwallet->GetSigningProvider(out.scriptPubKey);
         if (provider) {
-            UpdatePSBTOutput(HidingSigningProvider(provider, true, !bip32derivs), psbtx, i);
+            UpdatePSBTOutput(HidingSigningProvider(provider.get(), true, !bip32derivs), psbtx, i);
         }
     }
 

src/wallet/rpcwallet.cpp

@@ -557,7 +557,7 @@ static UniValue signmessage(const JSONRPCRequest& request)
     }
 
     CScript script_pub_key = GetScriptForDestination(*pkhash);
-    const SigningProvider* provider = pwallet->GetSigningProvider(script_pub_key);
+    std::unique_ptr<SigningProvider> provider = pwallet->GetSigningProvider(script_pub_key);
     if (!provider) {
         throw JSONRPCError(RPC_WALLET_ERROR, "Private key not available");
     }
@@ -2958,7 +2958,7 @@ static UniValue listunspent(const JSONRPCRequest& request)
                 entry.pushKV("label", i->second.name);
             }
 
-            const SigningProvider* provider = pwallet->GetSigningProvider(scriptPubKey);
+            std::unique_ptr<SigningProvider> provider = pwallet->GetSigningProvider(scriptPubKey);
             if (provider) {
                 if (scriptPubKey.IsPayToScriptHash()) {
                     const CScriptID& hash = CScriptID(boost::get<ScriptHash>(address));
@@ -2998,7 +2998,7 @@ static UniValue listunspent(const JSONRPCRequest& request)
         entry.pushKV("spendable", out.fSpendable);
         entry.pushKV("solvable", out.fSolvable);
         if (out.fSolvable) {
-            const SigningProvider* provider = pwallet->GetSigningProvider(scriptPubKey);
+            std::unique_ptr<SigningProvider> provider = pwallet->GetSigningProvider(scriptPubKey);
             if (provider) {
                 auto descriptor = InferDescriptor(scriptPubKey, *provider);
                 entry.pushKV("desc", descriptor->ToString());
@@ -3309,21 +3309,21 @@ UniValue signrawtransactionwithwallet(const JSONRPCRequest& request)
     // Parse the prevtxs array
     ParsePrevouts(request.params[1], nullptr, coins);
 
-    std::set<const SigningProvider*> providers;
+    std::set<std::shared_ptr<SigningProvider>> providers;
     for (const std::pair<COutPoint, Coin> coin_pair : coins) {
-        const SigningProvider* provider = pwallet->GetSigningProvider(coin_pair.second.out.scriptPubKey);
+        std::unique_ptr<SigningProvider> provider = pwallet->GetSigningProvider(coin_pair.second.out.scriptPubKey);
         if (provider) {
             providers.insert(std::move(provider));
         }
     }
     if (providers.size() == 0) {
         // When there are no available providers, use DUMMY_SIGNING_PROVIDER so we can check if the tx is complete
-        providers.insert(&DUMMY_SIGNING_PROVIDER);
+        providers.insert(std::make_shared<SigningProvider>());
     }
 
     UniValue result;
-    for (const SigningProvider* provider : providers) {
-        result = SignTransaction(mtx, provider, coins, request.params[2]);
+    for (std::shared_ptr<SigningProvider> provider : providers) {
+        result = SignTransaction(mtx, provider.get(), coins, request.params[2]);
     }
      return result;
 }
@@ -3680,12 +3680,12 @@ static UniValue DescribeWalletAddress(CWallet* pwallet, const CTxDestination& de
     UniValue ret(UniValue::VOBJ);
     UniValue detail = DescribeAddress(dest);
     CScript script = GetScriptForDestination(dest);
-    const SigningProvider* provider = nullptr;
+    std::unique_ptr<SigningProvider> provider = nullptr;
     if (pwallet) {
         provider = pwallet->GetSigningProvider(script);
     }
     ret.pushKVs(detail);
-    ret.pushKVs(boost::apply_visitor(DescribeWalletAddressVisitor(provider), dest));
+    ret.pushKVs(boost::apply_visitor(DescribeWalletAddressVisitor(provider.get()), dest));
     return ret;
 }
 
@@ -3774,7 +3774,7 @@ UniValue getaddressinfo(const JSONRPCRequest& request)
 
     CScript scriptPubKey = GetScriptForDestination(dest);
     ret.pushKV("scriptPubKey", HexStr(scriptPubKey.begin(), scriptPubKey.end()));
-    const SigningProvider* provider = pwallet->GetSigningProvider(scriptPubKey);
+    std::unique_ptr<SigningProvider> provider = pwallet->GetSigningProvider(scriptPubKey);
 
     isminetype mine = pwallet->IsMine(dest);
     ret.pushKV("ismine", bool(mine & ISMINE_SPENDABLE));

src/wallet/scriptpubkeyman.cpp

@@ -494,9 +494,9 @@ int64_t LegacyScriptPubKeyMan::GetTimeFirstKey() const
     return nTimeFirstKey;
 }
 
-const SigningProvider* LegacyScriptPubKeyMan::GetSigningProvider(const CScript& script) const
+std::unique_ptr<SigningProvider> LegacyScriptPubKeyMan::GetSigningProvider(const CScript& script) const
 {
-    return this;
+    return MakeUnique<LegacySigningProvider>(*this);
 }
 
 bool LegacyScriptPubKeyMan::CanProvide(const CScript& script, SignatureData& sigdata)

src/wallet/scriptpubkeyman.h

@@ -193,7 +193,7 @@ class ScriptPubKeyMan
 
     virtual const CKeyMetadata* GetMetadata(uint160 id) const { return nullptr; }
 
-    virtual const SigningProvider* GetSigningProvider(const CScript& script) const { return nullptr; }
+    virtual std::unique_ptr<SigningProvider> GetSigningProvider(const CScript& script) const { return nullptr; }
 
     /** Whether this ScriptPubKeyMan can provide a SigningProvider (via GetSigningProvider) that, combined with
       * sigdata, can produce a valid signature.
@@ -338,7 +338,7 @@ class LegacyScriptPubKeyMan : public ScriptPubKeyMan, public FillableSigningProv
 
     bool CanGetAddresses(bool internal = false) override;
 
-    const SigningProvider* GetSigningProvider(const CScript& script) const override;
+    std::unique_ptr<SigningProvider> GetSigningProvider(const CScript& script) const override;
 
     bool CanProvide(const CScript& script, SignatureData& sigdata) override;
 
@@ -440,4 +440,20 @@ class LegacyScriptPubKeyMan : public ScriptPubKeyMan, public FillableSigningProv
     std::set<CKeyID> GetKeys() const override;
 };
 
+/** Wraps a LegacyScriptPubKeyMan so that it can be returned in a new unique_ptr */
+class LegacySigningProvider : public SigningProvider
+{
+private:
+    const LegacyScriptPubKeyMan& spk_man;
+public:
+    LegacySigningProvider(const LegacyScriptPubKeyMan& spk_man) : spk_man(spk_man) {}
+
+    bool GetCScript(const CScriptID &scriptid, CScript& script) const override { return spk_man.GetCScript(scriptid, script); }
+    bool HaveCScript(const CScriptID &scriptid) const override { return spk_man.HaveCScript(scriptid); }
+    bool GetPubKey(const CKeyID &address, CPubKey& pubkey) const override { return spk_man.GetPubKey(address, pubkey); }
+    bool GetKey(const CKeyID &address, CKey& key) const override { return spk_man.GetKey(address, key); }
+    bool HaveKey(const CKeyID &address) const override { return spk_man.HaveKey(address); }
+    bool GetKeyOrigin(const CKeyID& keyid, KeyOriginInfo& info) const override { return spk_man.GetKeyOrigin(keyid, info); }
+};
+
 #endif // BITCOIN_WALLET_SCRIPTPUBKEYMAN_H

src/wallet/wallet.cpp

@@ -1338,7 +1338,7 @@ bool CWallet::DummySignInput(CTxIn &tx_in, const CTxOut &txout, bool use_max_sig
     const CScript& scriptPubKey = txout.scriptPubKey;
     SignatureData sigdata;
 
-    const SigningProvider* provider = GetSigningProvider(scriptPubKey);
+    std::unique_ptr<SigningProvider> provider = GetSigningProvider(scriptPubKey);
     if (!provider) {
         // We don't know about this scriptpbuKey;
         return false;
@@ -2094,7 +2094,7 @@ void CWallet::AvailableCoins(interfaces::Chain::Lock& locked_chain, std::vector<
                 continue;
             }
 
-            const SigningProvider* provider = GetSigningProvider(wtx.tx->vout[i].scriptPubKey);
+            std::unique_ptr<SigningProvider> provider = GetSigningProvider(wtx.tx->vout[i].scriptPubKey);
 
             bool solvable = provider ? IsSolvable(*provider, wtx.tx->vout[i].scriptPubKey) : false;
             bool spendable = ((mine & ISMINE_SPENDABLE) != ISMINE_NO) || (((mine & ISMINE_WATCH_ONLY) != ISMINE_NO) && (coinControl && coinControl->fAllowWatchOnly && solvable));
@@ -2328,7 +2328,7 @@ bool CWallet::SignTransaction(CMutableTransaction& tx)
         const CAmount& amount = mi->second.tx->vout[input.prevout.n].nValue;
         SignatureData sigdata;
 
-        const SigningProvider* provider = GetSigningProvider(scriptPubKey);
+        std::unique_ptr<SigningProvider> provider = GetSigningProvider(scriptPubKey);
         if (!provider) {
             // We don't know about this scriptpbuKey;
             return false;
@@ -2792,7 +2792,7 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std
                 const CScript& scriptPubKey = coin.txout.scriptPubKey;
                 SignatureData sigdata;
 
-                const SigningProvider* provider = GetSigningProvider(scriptPubKey);
+                std::unique_ptr<SigningProvider> provider = GetSigningProvider(scriptPubKey);
                 if (!provider) {
                     // We don't know about this scriptpbuKey;
                     return false;
@@ -4086,13 +4086,13 @@ ScriptPubKeyMan* CWallet::GetScriptPubKeyMan(const uint256& id) const
     return nullptr;
 }
 
-const SigningProvider* CWallet::GetSigningProvider(const CScript& script) const
+std::unique_ptr<SigningProvider> CWallet::GetSigningProvider(const CScript& script) const
 {
     SignatureData sigdata;
     return GetSigningProvider(script, sigdata);
 }
 
-const SigningProvider* CWallet::GetSigningProvider(const CScript& script, SignatureData& sigdata) const
+std::unique_ptr<SigningProvider> CWallet::GetSigningProvider(const CScript& script, SignatureData& sigdata) const
 {
     for (const auto& spk_man_pair : m_spk_managers) {
         if (spk_man_pair.second->CanProvide(script, sigdata)) {

src/wallet/wallet.h

@@ -1111,8 +1111,8 @@ class CWallet final : public WalletStorage, private interfaces::Chain::Notificat
     ScriptPubKeyMan* GetScriptPubKeyMan(const uint256& id) const;
 
     //! Get the SigningProvider for a script
-    const SigningProvider* GetSigningProvider(const CScript& script) const;
-    const SigningProvider* GetSigningProvider(const CScript& script, SignatureData& sigdata) const;
+    std::unique_ptr<SigningProvider> GetSigningProvider(const CScript& script) const;
+    std::unique_ptr<SigningProvider> GetSigningProvider(const CScript& script, SignatureData& sigdata) const;
 
     //! Get the LegacyScriptPubKeyMan which is used for all types, internal, and external.
     LegacyScriptPubKeyMan* GetLegacyScriptPubKeyMan() const;