Move direct calls to MessageSign into new SignMessage functions in CW…

…allet and ScriptPubKeyMan

Summary:
Instead of getting a SigningProvider and then going to MessageSign,
have ScriptPubKeyMan handle the message signing internally.

Backport of Core [[bitcoin/bitcoin#18115 | PR18115]] part [7/9] : bitcoin/bitcoin@6a9c429

Depends on D8101 and D8098

Test Plan:
  ninja all check-all

Reviewers: #bitcoin_abc, majcosta

Reviewed By: #bitcoin_abc, majcosta

Differential Revision: https://reviews.bitcoinabc.org/D8102

src/interfaces/wallet.cpp

@@ -144,6 +144,11 @@ namespace {
             }
             return false;
         }
+        SigningResult signMessage(const std::string &message,
+                                  const PKHash &pkhash,
+                                  std::string &str_sig) override {
+            return m_wallet->SignMessage(message, pkhash, str_sig);
+        }
         bool isSpendable(const CTxDestination &dest) override {
             return m_wallet->IsMine(dest) & ISMINE_SPENDABLE;
         }

src/interfaces/wallet.h

@@ -12,6 +12,7 @@
 #include <script/standard.h>           // For CTxDestination
 #include <support/allocators/secure.h> // For SecureString
 #include <ui_interface.h>              // For ChangeType
+#include <util/message.h>
 
 #include <cstdint>
 #include <functional>
@@ -105,6 +106,11 @@ class Wallet {
     virtual bool getPrivKey(const CScript &script, const CKeyID &address,
                             CKey &key) = 0;
 
+    //! Sign message
+    virtual SigningResult signMessage(const std::string &message,
+                                      const PKHash &pkhash,
+                                      std::string &str_sig) = 0;
+
     //! Return whether wallet has private key.
     virtual bool isSpendable(const CTxDestination &dest) = 0;
 

src/qt/signverifymessagedialog.cpp

@@ -139,23 +139,29 @@ void SignVerifyMessageDialog::on_signMessageButton_SM_clicked() {
         return;
     }
 
-    CKey key;
-    if (!model->wallet().getPrivKey(GetScriptForDestination(destination),
-                                    CKeyID(*pkhash), key)) {
-        ui->statusLabel_SM->setStyleSheet("QLabel { color: red; }");
-        ui->statusLabel_SM->setText(
-            tr("Private key for the entered address is not available."));
-        return;
-    }
-
     const std::string &message =
         ui->messageIn_SM->document()->toPlainText().toStdString();
     std::string signature;
+    SigningResult res =
+        model->wallet().signMessage(message, *pkhash, signature);
+
+    QString error;
+    switch (res) {
+        case SigningResult::OK:
+            error = tr("No error");
+            break;
+        case SigningResult::PRIVATE_KEY_NOT_AVAILABLE:
+            error = tr("Private key for the entered address is not available.");
+            break;
+        case SigningResult::SIGNING_FAILED:
+            error = tr("Message signing failed.");
+            break;
+            // no default case, so the compiler can warn about missing cases
+    }
 
-    if (!MessageSign(key, message, signature)) {
+    if (res != SigningResult::OK) {
         ui->statusLabel_SM->setStyleSheet("QLabel { color: red; }");
-        ui->statusLabel_SM->setText(QString("<nobr>") +
-                                    tr("Message signing failed.") +
+        ui->statusLabel_SM->setText(QString("<nobr>") + error +
                                     QString("</nobr>"));
         return;
     }

src/util/message.cpp

@@ -72,3 +72,16 @@ uint256 MessageHash(const std::string &message) {
 
     return hasher.GetHash();
 }
+
+std::string SigningResultString(const SigningResult res) {
+    switch (res) {
+        case SigningResult::OK:
+            return "No error";
+        case SigningResult::PRIVATE_KEY_NOT_AVAILABLE:
+            return "Private key not available";
+        case SigningResult::SIGNING_FAILED:
+            return "Sign failed";
+            // no default case, so the compiler can warn about missing cases
+    }
+    assert(false);
+}

src/util/message.h

@@ -44,6 +44,12 @@ enum class MessageVerificationResult {
     OK
 };
 
+enum class SigningResult {
+    OK, //!< No error
+    PRIVATE_KEY_NOT_AVAILABLE,
+    SIGNING_FAILED,
+};
+
 /**
  * Verify a signed message.
  * @param[in] chain params to be used to interpret the address.
@@ -74,4 +80,6 @@ bool MessageSign(const CKey &privkey, const std::string &message,
  */
 uint256 MessageHash(const std::string &message);
 
+std::string SigningResultString(const SigningResult res);
+
 #endif // BITCOIN_UTIL_MESSAGE_H

src/wallet/rpcwallet.cpp

@@ -620,23 +620,13 @@ static UniValue signmessage(const Config &config,
         throw JSONRPCError(RPC_TYPE_ERROR, "Address does not refer to key");
     }
 
-    CScript script_pub_key = GetScriptForDestination(*pkhash);
-    std::unique_ptr<SigningProvider> provider =
-        pwallet->GetSigningProvider(script_pub_key);
-    if (!provider) {
-        throw JSONRPCError(RPC_WALLET_ERROR, "Private key not available");
-    }
-
-    CKey key;
-    CKeyID keyID(*pkhash);
-    if (!provider->GetKey(keyID, key)) {
-        throw JSONRPCError(RPC_WALLET_ERROR, "Private key not available");
-    }
-
     std::string signature;
-
-    if (!MessageSign(key, strMessage, signature)) {
-        throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Sign failed");
+    SigningResult err = pwallet->SignMessage(strMessage, *pkhash, signature);
+    if (err == SigningResult::SIGNING_FAILED) {
+        throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY,
+                           SigningResultString(err));
+    } else if (err != SigningResult::OK) {
+        throw JSONRPCError(RPC_WALLET_ERROR, SigningResultString(err));
     }
 
     return signature;

src/wallet/scriptpubkeyman.cpp

@@ -465,6 +465,21 @@ bool LegacyScriptPubKeyMan::SignTransaction(
     return ::SignTransaction(tx, this, coins, sighash, input_errors);
 }
 
+SigningResult LegacyScriptPubKeyMan::SignMessage(const std::string &message,
+                                                 const PKHash &pkhash,
+                                                 std::string &str_sig) const {
+    CKeyID key_id(pkhash);
+    CKey key;
+    if (!GetKey(key_id, key)) {
+        return SigningResult::PRIVATE_KEY_NOT_AVAILABLE;
+    }
+
+    if (MessageSign(key, message, str_sig)) {
+        return SigningResult::OK;
+    }
+    return SigningResult::SIGNING_FAILED;
+}
+
 TransactionError
 LegacyScriptPubKeyMan::FillPSBT(PartiallySignedTransaction &psbtx,
                                 SigHashType sighash_type, bool sign,

src/wallet/scriptpubkeyman.h

@@ -9,6 +9,7 @@
 #include <script/signingprovider.h>
 #include <script/standard.h>
 #include <util/error.h>
+#include <util/message.h>
 #include <wallet/crypter.h>
 #include <wallet/ismine.h>
 #include <wallet/walletdb.h>
@@ -265,6 +266,12 @@ class ScriptPubKeyMan {
                     std::map<int, std::string> &input_errors) const {
         return false;
     }
+    /** Sign a message with the given script */
+    virtual SigningResult SignMessage(const std::string &message,
+                                      const PKHash &pkhash,
+                                      std::string &str_sig) const {
+        return SigningResult::SIGNING_FAILED;
+    };
     /**
      * Adds script and derivation path information to a PSBT, and optionally
      * signs it.
@@ -447,6 +454,8 @@ class LegacyScriptPubKeyMan : public ScriptPubKeyMan,
     SignTransaction(CMutableTransaction &tx,
                     const std::map<COutPoint, Coin> &coins, SigHashType sighash,
                     std::map<int, std::string> &input_errors) const override;
+    SigningResult SignMessage(const std::string &message, const PKHash &pkhash,
+                              std::string &str_sig) const override;
     TransactionError
     FillPSBT(PartiallySignedTransaction &psbt,
              SigHashType sighash_type = SigHashType().withForkId(),

src/wallet/wallet.cpp

@@ -2879,6 +2879,19 @@ TransactionError CWallet::FillPSBT(PartiallySignedTransaction &psbtx,
     return TransactionError::OK;
 }
 
+SigningResult CWallet::SignMessage(const std::string &message,
+                                   const PKHash &pkhash,
+                                   std::string &str_sig) const {
+    SignatureData sigdata;
+    CScript script_pub_key = GetScriptForDestination(pkhash);
+    for (const auto &spk_man_pair : m_spk_managers) {
+        if (spk_man_pair.second->CanProvide(script_pub_key, sigdata)) {
+            return spk_man_pair.second->SignMessage(message, pkhash, str_sig);
+        }
+    }
+    return SigningResult::PRIVATE_KEY_NOT_AVAILABLE;
+}
+
 bool CWallet::FundTransaction(CMutableTransaction &tx, Amount &nFeeRet,
                               int &nChangePosInOut, bilingual_str &error,
                               bool lockUnspents,

src/wallet/wallet.h

@@ -15,6 +15,7 @@
 #include <psbt.h>
 #include <tinyformat.h>
 #include <ui_interface.h>
+#include <util/message.h>
 #include <util/strencodings.h>
 #include <util/system.h>
 #include <util/translation.h>
@@ -1106,6 +1107,8 @@ class CWallet final : public WalletStorage,
                          const std::map<COutPoint, Coin> &coins,
                          SigHashType sighash,
                          std::map<int, std::string> &input_errors) const;
+    SigningResult SignMessage(const std::string &message, const PKHash &pkhash,
+                              std::string &str_sig) const;
 
     /**
      * Fills out a PSBT with information from the wallet. Fills in UTXOs if we