Deduplicate the message signing code

The logic of signing a message was duplicated in 3 places: src/qt/signverifymessagedialog.cpp SignVerifyMessageDialog::on_signMessageButton_SM_clicked() src/rpc/misc.cpp signmessagewithprivkey() src/wallet/rpcwallet.cpp signmessage() Move the logic into src/util/message.cpp MessageSign() and call it from all the 3 places.
syscoin-core · Feb 14, 2020 · f8f0d98 · f8f0d98
1 parent 2ce3447
commit f8f0d98
Show file tree

Hide file tree

Showing 6 changed files with 87 additions and 23 deletions.
diff --git a/src/qt/signverifymessagedialog.cpp b/src/qt/signverifymessagedialog.cpp
@@ -11,7 +11,7 @@
 #include <qt/walletmodel.h>
 
 #include <key_io.h>
-#include <util/message.h> // For strMessageMagic, MessageVerify()
+#include <util/message.h> // For MessageSign(), MessageVerify()
 #include <wallet/wallet.h>
 
 #include <vector>
@@ -141,13 +141,10 @@ void SignVerifyMessageDialog::on_signMessageButton_SM_clicked()
         return;
     }
 
-    CHashWriter ss(SER_GETHASH, 0);
-    ss << strMessageMagic;
-    ss << ui->messageIn_SM->document()->toPlainText().toStdString();
+    const std::string& message = ui->messageIn_SM->document()->toPlainText().toStdString();
+    std::string signature;
 
-    std::vector<unsigned char> vchSig;
-    if (!key.SignCompact(ss.GetHash(), vchSig))
-    {
+    if (!MessageSign(key, message, signature)) {
         ui->statusLabel_SM->setStyleSheet("QLabel { color: red; }");
         ui->statusLabel_SM->setText(QString("<nobr>") + tr("Message signing failed.") + QString("</nobr>"));
         return;
@@ -156,7 +153,7 @@ void SignVerifyMessageDialog::on_signMessageButton_SM_clicked()
     ui->statusLabel_SM->setStyleSheet("QLabel { color: green; }");
     ui->statusLabel_SM->setText(QString("<nobr>") + tr("Message signed.") + QString("</nobr>"));
 
-    ui->signatureOut_SM->setText(QString::fromStdString(EncodeBase64(vchSig.data(), vchSig.size())));
+    ui->signatureOut_SM->setText(QString::fromStdString(signature));
 }
 
 void SignVerifyMessageDialog::on_copySignatureButton_SM_clicked()

diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp
@@ -11,7 +11,7 @@
 #include <rpc/util.h>
 #include <script/descriptor.h>
 #include <util/check.h>
-#include <util/message.h> // For strMessageMagic, MessageVerify()
+#include <util/message.h> // For MessageSign(), MessageVerify()
 #include <util/strencodings.h>
 #include <util/system.h>
 
@@ -322,15 +322,13 @@ static UniValue signmessagewithprivkey(const JSONRPCRequest& request)
         throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid private key");
     }
 
-    CHashWriter ss(SER_GETHASH, 0);
-    ss << strMessageMagic;
-    ss << strMessage;
+    std::string signature;
 
-    std::vector<unsigned char> vchSig;
-    if (!key.SignCompact(ss.GetHash(), vchSig))
+    if (!MessageSign(key, strMessage, signature)) {
         throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Sign failed");
+    }
 
-    return EncodeBase64(vchSig.data(), vchSig.size());
+    return signature;
 }
 
 static UniValue setmocktime(const JSONRPCRequest& request)

diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp
@@ -5,18 +5,20 @@
 #include <util/system.h>
 
 #include <clientversion.h>
+#include <key.h> // For CKey
 #include <optional.h>
 #include <sync.h>
 #include <test/util/setup_common.h>
 #include <test/util/str.h>
-#include <util/message.h> // For MessageVerify()
+#include <util/message.h> // For MessageSign(), MessageVerify()
 #include <util/moneystr.h>
 #include <util/strencodings.h>
 #include <util/string.h>
 #include <util/time.h>
 #include <util/spanparsing.h>
 #include <util/vector.h>
 
+#include <array>
 #include <stdint.h>
 #include <thread>
 #include <univalue.h>
@@ -2026,6 +2028,42 @@ BOOST_AUTO_TEST_CASE(test_tracked_vector)
     BOOST_CHECK_EQUAL(v8[2].copies, 0);
 }
 
+BOOST_AUTO_TEST_CASE(message_sign)
+{
+    const std::array<unsigned char, 32> privkey_bytes = {
+        // just some random data
+        // derived address from this private key: 15CRxFdyRpGZLW9w8HnHvVduizdL5jKNbs
+        0xD9, 0x7F, 0x51, 0x08, 0xF1, 0x1C, 0xDA, 0x6E,
+        0xEE, 0xBA, 0xAA, 0x42, 0x0F, 0xEF, 0x07, 0x26,
+        0xB1, 0xF8, 0x98, 0x06, 0x0B, 0x98, 0x48, 0x9F,
+        0xA3, 0x09, 0x84, 0x63, 0xC0, 0x03, 0x28, 0x66
+    };
+
+    const std::string message = "Trust no one";
+
+    const std::string expected_signature =
+        "IPojfrX2dfPnH26UegfbGQQLrdK844DlHq5157/P6h57WyuS/Qsl+h/WSVGDF4MUi4rWSswW38oimDYfNNUBUOk=";
+
+    CKey privkey;
+    std::string generated_signature;
+
+    BOOST_REQUIRE_MESSAGE(!privkey.IsValid(),
+        "Confirm the private key is invalid");
+
+    BOOST_CHECK_MESSAGE(!MessageSign(privkey, message, generated_signature),
+        "Sign with an invalid private key");
+
+    privkey.Set(privkey_bytes.begin(), privkey_bytes.end(), true);
+
+    BOOST_REQUIRE_MESSAGE(privkey.IsValid(),
+        "Confirm the private key is valid");
+
+    BOOST_CHECK_MESSAGE(MessageSign(privkey, message, generated_signature),
+        "Sign with a valid private key");
+
+    BOOST_CHECK_EQUAL(expected_signature, generated_signature);
+}
+
 BOOST_AUTO_TEST_CASE(message_verify)
 {
     BOOST_CHECK_EQUAL(

diff --git a/src/util/message.cpp b/src/util/message.cpp
@@ -4,6 +4,7 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <hash.h>            // For CHashWriter
+#include <key.h>             // For CKey
 #include <key_io.h>          // For DecodeDestination()
 #include <pubkey.h>          // For CPubKey
 #include <script/standard.h> // For CTxDestination, IsValidDestination(), PKHash
@@ -51,3 +52,23 @@ MessageVerificationResult MessageVerify(
 
     return MessageVerificationResult::OK;
 }
+
+bool MessageSign(
+    const CKey& privkey,
+    const std::string& message,
+    std::string& signature)
+{
+    CHashWriter ss(SER_GETHASH, 0);
+    ss << strMessageMagic;
+    ss << message;
+
+    std::vector<unsigned char> signature_bytes;
+
+    if (!privkey.SignCompact(ss.GetHash(), signature_bytes)) {
+        return false;
+    }
+
+    signature = EncodeBase64(signature_bytes.data(), signature_bytes.size());
+
+    return true;
+}
diff --git a/src/util/message.h b/src/util/message.h
@@ -6,6 +6,8 @@
 #ifndef BITCOIN_UTIL_MESSAGE_H
 #define BITCOIN_UTIL_MESSAGE_H
 
+#include <key.h> // For CKey
+
 #include <string>
 
 extern const std::string strMessageMagic;
@@ -46,4 +48,14 @@ MessageVerificationResult MessageVerify(
     const std::string& signature,
     const std::string& message);
 
+/** Sign a message.
+ * @param[in] privkey Private key to sign with.
+ * @param[in] message The message to sign.
+ * @param[out] signature Signature, base64 encoded, only set if true is returned.
+ * @return true if signing was successful. */
+bool MessageSign(
+    const CKey& privkey,
+    const std::string& message,
+    std::string& signature);
+
 #endif // BITCOIN_UTIL_MESSAGE_H
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
@@ -19,7 +19,7 @@
 #include <script/sign.h>
 #include <util/bip32.h>
 #include <util/fees.h>
-#include <util/message.h> // For strMessageMagic
+#include <util/message.h> // For MessageSign()
 #include <util/moneystr.h>
 #include <util/string.h>
 #include <util/system.h>
@@ -576,15 +576,13 @@ static UniValue signmessage(const JSONRPCRequest& request)
         throw JSONRPCError(RPC_WALLET_ERROR, "Private key not available");
     }
 
-    CHashWriter ss(SER_GETHASH, 0);
-    ss << strMessageMagic;
-    ss << strMessage;
+    std::string signature;
 
-    std::vector<unsigned char> vchSig;
-    if (!key.SignCompact(ss.GetHash(), vchSig))
+    if (!MessageSign(key, strMessage, signature)) {
         throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Sign failed");
+    }
 
-    return EncodeBase64(vchSig.data(), vchSig.size());
+    return signature;
 }
 
 static UniValue getreceivedbyaddress(const JSONRPCRequest& request)